[jdk17] RFR: 8261147: C2: Node is wrongly marked as reduction resulting in a wrong execution due to wrong vector instructions

Vladimir Kozlov kvn at openjdk.java.net
Thu Jul 8 16:35:53 UTC 2021


On Thu, 8 Jul 2021 10:36:28 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

> C2 produces a wrong result for the testcase due to treating a non-reduction `AddL` node wrongly as a reduction (`Node::is_reduction()` is true). This node is later part of a pack in the superword algorithm which wrongly treats it as a reduction. As a result, we create an `AddReductionVL` instead of an `AddVL` node, letting the test fail.
> 
> The wrong reduction can be traced back to `AddNode::Ideal` where we create a clone of an `AddL` node which is marked as a reduction. However, the clone itself is no longer a reduction but is still marked as such. `Node::clone()` simply copies the `Flag_is_reduction` flag in `Node::_flags`:
> https://github.com/openjdk/jdk17/blob/4f707591754e5e7f747d1d0a47f78f49060771c2/src/hotspot/share/opto/addnode.cpp#L181-L196
> 
> I don't think we need to clone the `Flag_is_reduction` flag by default in `Node::clone()`. I went through the uses of `Node::clone()` and I think we only need to clone the flag when copying an entire loop body in `PhaseIdealLoop::clone_loop()`.  That's what I propose as a fix. This approach also avoids the need to think about reductions in future uses of `Node::clone()` with a special handling for it as it would be required in `AddNode::Ideal` etc.
> 
> I did some performance testing with some common benchmarks which looked good. I also ran the follwing JTreg tests which showed the same number of newly created vectors with and without the fix:
> 
> jtreg -testjdk:jdk-18/fastdebug -va -javaoptions:'-server -Xbatch -XX:-TieredCompilation -XX:CICompilerCount=1 -XX:+TraceNewVectors' -J-Djavatest.maxOutputSize=1000000 compiler/c2/cr6340864/ compiler/codegen/ compiler/loopopts/superword/ compiler/vectorization > new_vects.log
> grep "new Vector node:" new_vects.log | wc
>   20190  521206 4398994
> 
> 
> Thanks,
> Christian

Good.

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

Marked as reviewed by kvn (Reviewer).

PR: https://git.openjdk.java.net/jdk17/pull/231


More information about the hotspot-compiler-dev mailing list