On Mon, 15 May 2023 08:29:31 GMT, JoKern65 <d...@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
