RFR: 8276105: C2: Conv(D|F)2(I|L)Nodes::Ideal should handle rounding correctly

[Tobias Hartmann]

On Fri, 29 Oct 2021 16:38:11 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

> Happens now in master:
> 
> 
> $ CONF=linux-x86-server-fastdebug make run-test TEST=compiler/loopopts/superword/CoLocatePack.java TEST_VM_OPTS="-XX:UseAVX=0 -XX:UseSSE=0"
> ...
> 
> CompileCommand: compileonly compiler/loopopts/superword/CoLocatePack.test bool compileonly = true
>  191 ConvF2L === _ 714 [[ 193 ]] !jvms: CoLocatePack::test @ bci:30 (line 70)
> # To suppress the following error report, specify this argument
> # after -XX: or in .hotspotrc: SuppressErrorAt=/phaseX.cpp:1128
> #
> # A fatal error has been detected by the Java Runtime Environment:
> #
> # Internal Error (/home/shade/trunks/jdk/src/hotspot/share/opto/phaseX.cpp:1128), pid=1717516, tid=1717532
> # fatal error: modified node was not processed by IGVN.transform_old()
> 
> 
> After JDK-8266950 (always `strictfp`), the paths in `Conv(D|F)2(I|L)Nodes::Ideal`-s start to be taken more frequently to round float/double inputs when low SSE is enabled. On those paths, we call `set_req` to rewire current node, but we still return `NULL` from `::Ideal`. I believe that is incorrect, as per `node.cpp` explanation: `NULL` indicates no graph change was done, and `this` should be returned when modification happened. So GVN predictably barfs.
> 
> Additional testing:
>  - [x] Linux x86_32 `tier1` with `-XX:UseAVX=0 -XX:UseSSE=0` (now pass)
>  - [x] Linux x86_32 `tier1` default (still pass)
>  - [x] Linux x86_64 `tier1` default

Looks good. Is it worth adding the flag combinations to the test?

src/hotspot/share/opto/convertnode.cpp line 148:

> 146: Node *ConvD2LNode::Ideal(PhaseGVN *phase, bool can_reshape) {
> 147:   if (in(1)->Opcode() == Op_RoundDouble) {
> 148:     set_req(1,in(1)->in(1));

Whitespace after `,` is missing.

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

Marked as reviewed by thartmann (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/6176