RFR: JDK-8306304: Fix xlc17 clang warnings in ppc and aix code [v2]

Kim Barrett kbarrett at openjdk.org
Mon May 15 09:37:46 UTC 2023


On Mon, 15 May 2023 08:29:31 GMT, JoKern65 <duke at openjdk.org> wrote:

>> src/hotspot/cpu/ppc/ppc.ad line 11444:
>> 
>>> 11442:   effect(KILL cr0);
>>> 11443:   ins_cost(DEFAULT_COST * 5);
>>> 11444:   size((VM_Version::has_brw() ? 16 : 20));
>> 
>> What is it complaining about here?
>
> /data/d042520/xlc17/jdk/src/hotspot/cpu/ppc/c1_LIRGenerator_ppc.cpp:426:97: error: shifting a negative signed value is undefined [-Werror,-Wshift-negative-value]
> I reverted my change in c1_LIRGenerator_ppc.cpp and added shift-negative-value to the DISABLED_WARNINGS_clang in CompileJvm.gmk.
> 
> ad_ppc.cpp:18388:10: error: converting the result of '?:' with integer constants to a boolean always evaluates to 'true' [-Werror,-Wtautological-constant-compare]
>   assert(VerifyOops || MachNode::size(ra_) <= VM_Version::has_brw() ? 16 : 20, "bad fixed size");
>          ^
> Should I also add tautological-constant-compare to DISABLED_WARNINGS_clang in CompileJvm.gmk or where else?

I see, so `size` is kind of macro-like, and is just textually splicing its argument expression into another expression.
And without the added parens the resulting full expression for the assert isn't checking what's intended, due
to operator precedence.

This is in generated source; it might be better to find the code generator (somewhere in adlc) and change it
to add appropriate parens, as there may be other similar places (both here and for other platforms) that
aren't doing what's intended but are not triggering warnings.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13953#discussion_r1193579404


More information about the hotspot-dev mailing list