RFR: 8240615: is_power_of_2() has Undefined Behaviour and is inconsistent

[Andrew Haley]

On 3/6/20 2:48 PM, Andrew Haley wrote:
>> The predicate for Pow2 and NotPow2 will both be false because of this
>> change. Is that a problem?
> Probably. I'll do some more digging.

The pattern genertates a bitset instruction. It'll generate slightly
less efficient code after my change. Maybe.

It's pretty damm hard to persuade C2 actually to use this pattern, but
this works:

    @Benchmark
    public void test_btsL_mem_imm(BenchmarkState state) {
        for (int i = 0; i < 1000; i += 3) {
            state.array[i] |= 0x4000_0000_0000_0000l;
            state.array[i+1] |= 0x8000_0000_0000_0000l;
            state.array[i+2] |= 0x1000_0000_0000_0000l;
        }
    }

These three |= ops turn into:

  0x00007fe65cc58f40:   bts    QWORD PTR [r8+r10*8+0x10],0x3e;*lastore {reexecute=0 rethrow=0 return_oop=0}
  0x00007fe65cc58f47:   or     QWORD PTR [r8+r10*8+0x18],rdi;*lastore {reexecute=0 rethrow=0 return_oop=0}
  0x00007fe65cc58f4c:   bts    QWORD PTR [r8+r10*8+0x20],0x3c;*lastore {reexecute=0 rethrow=0 return_oop=0}

There is a very slight pessimization in that RDI is now occupied, so
the register pressure is slightly increased, and there's the cost of a
64-bit load immediate operation.

We can get back to the status quo with this patch:

--- a/src/hotspot/cpu/x86/x86_64.ad     Thu Mar 05 17:56:08 2020 +0000
+++ b/src/hotspot/cpu/x86/x86_64.ad     Fri Mar 06 17:06:20 2020 +0000
@@ -3120,7 +3120,7 @@

 operand immL_Pow2()
 %{
-  predicate(is_power_of_2(n->get_long()));
+  predicate(is_power_of_2((julong)n->get_long()));
   match(ConL);

   op_cost(15);
@@ -3130,7 +3130,7 @@

 operand immL_NotPow2()
 %{
-  predicate(is_power_of_2(~n->get_long()));
+  predicate(is_power_of_2((julong)~n->get_long()));
   match(ConL);

   op_cost(15);

  0x00007fc688bf8010:   bts    QWORD PTR [r10+r8*8+0x10],0x3e;*lastore {reexecute=0 rethrow=0 return_oop=0}
  0x00007fc688bf8017:   bts    QWORD PTR [r10+r8*8+0x18],0x3f;*lastore {reexecute=0 rethrow=0 return_oop=0}
  0x00007fc688bf801e:   bts    QWORD PTR [r10+r8*8+0x20],0x3c;*lastore {reexecute=0 rethrow=0 return_oop=0}

There will be more examples of this, stuff that just works "by
accident". If we want the previous weird behaviour we could explictly
special-case that behaviour and test it. I'd rather not...

-- 
Andrew Haley  (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671