On Fri, 12 May 2023 21:51:59 GMT, Kim Barrett <[email protected]> wrote:
>> src/hotspot/cpu/ppc/c1_LIRGenerator_ppc.cpp line 426:
>>
>>> 424: // Missing test if instr is commutative and if we should swap.
>>> 425: if (right.value()->type()->as_LongConstant() &&
>>> 426: (x->op() == Bytecodes::_lsub &&
>>> right.value()->type()->as_LongConstant()->value() == -32768 ) ) {
>>
>> I would prefer a shifted value here as it's usually more readable. If the
>> compiler is being stubborn in its warnings, a comment explaining the magic
>> value would be fine too.
>
> What is the warning here? Note that we've already turned off
> `-Wshift-negative-value` for gcc and xlc
> (but not for clang, for some reason). See `# Disabled warnings` in
> CompileJvm.gmk.
I think disabling the warning is fine. Alternatively, we could `#define
MIN_INT16 -32768` somewhere or introduce `const int16_t min_int16 = (int16_t)1
<< (sizeof(int16_t)*BitsPerByte-1);`. What do you prefer, Kim?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13953#discussion_r1193762594
