Skip to content

Commit

Permalink
8279622: C2: miscompilation of map pattern as a vector reduction
Browse files Browse the repository at this point in the history
Backport-of: 6fcd322258e0cce3724a4a8dc18f7802018a7cc9
  • Loading branch information
GoeLin committed Jun 24, 2022
1 parent d90fd6f commit 47e478d
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 0 deletions.
9 changes: 9 additions & 0 deletions src/hotspot/share/opto/loopTransform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1557,6 +1557,15 @@ void PhaseIdealLoop::insert_pre_post_loops(IdealLoopTree *loop, Node_List &old_n
set_idom(new_pre_exit, pre_end, dd_main_head);
set_loop(new_pre_exit, outer_loop->_parent);

if (peel_only) {
// Nodes in the peeled iteration that were marked as reductions within the
// original loop might not be reductions within their new outer loop.
for (uint i = 0; i < loop->_body.size(); i++) {
Node* n = old_new[loop->_body[i]->_idx];
n->remove_flag(Node::Flag_is_reduction);
}
}

// Step B2: Build a zero-trip guard for the main-loop. After leaving the
// pre-loop, the main-loop may not execute at all. Later in life this
// zero-trip guard will become the minimum-trip guard when we unroll
Expand Down
11 changes: 11 additions & 0 deletions src/hotspot/share/opto/loopnode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3409,6 +3409,17 @@ uint IdealLoopTree::est_loop_flow_merge_sz() const {
return 0;
}

#ifdef ASSERT
bool IdealLoopTree::has_reduction_nodes() const {
for (uint i = 0; i < _body.size(); i++) {
if (_body[i]->is_reduction()) {
return true;
}
}
return false;
}
#endif // ASSERT

#ifndef PRODUCT
//------------------------------dump_head--------------------------------------
// Dump 1 liner for loop header info
Expand Down
5 changes: 5 additions & 0 deletions src/hotspot/share/opto/loopnode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,11 @@ class IdealLoopTree : public ResourceObj {

void remove_main_post_loops(CountedLoopNode *cl, PhaseIdealLoop *phase);

#ifdef ASSERT
// Tell whether the body contains nodes marked as reductions.
bool has_reduction_nodes() const;
#endif // ASSERT

#ifndef PRODUCT
void dump_head() const; // Dump loop head only
void dump() const; // Dump this loop recursively
Expand Down
2 changes: 2 additions & 0 deletions src/hotspot/share/opto/superword.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ bool SuperWord::transform_loop(IdealLoopTree* lpt, bool do_optimization) {
return false; // skip malformed counted loop
}

assert(!lpt->has_reduction_nodes() || cl->is_reduction_loop(),
"non-reduction loop contains reduction nodes");
bool post_loop_allowed = (PostLoopMultiversioning && Matcher::has_predicated_vectors() && cl->is_post_loop());
if (post_loop_allowed) {
if (cl->is_reduction_loop()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Copyright (c) 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

/**
* @test
* @bug 8279622
* @summary Test that reduction nodes peeled out of an inner loop are not
* vectorized as reductions within the outer loop.
* @library /test/lib
* @comment The test is run with -XX:LoopUnrollLimit=32 to prevent unrolling
* from fully replacing vectorization.
* @run main/othervm -Xbatch -XX:LoopUnrollLimit=32
* compiler.loopopts.superword.TestPeeledReductionNode
*/
package compiler.loopopts.superword;

import jdk.test.lib.Asserts;

public class TestPeeledReductionNode {
static final int N = 32;
static final int M = 65; // Must be odd and >= 65 to trigger the failure.
static final int INPUT = 0b0000_0000_0000_0000_0000_0000_0000_0001;
static final int MASK = 0b0000_0000_1000_0000_0000_0000_0000_0000;
static final int EXPECTED = (M % 2 == 0 ? INPUT : INPUT ^ MASK);
static int mask = 0;
public static void main(String[] args) {
int r[] = new int[N];
for (int i = 0; i < N; i++) {
r[i] = INPUT;
}
// Trigger the relevant OSR compilation and set
// TestPeeledReductionNode.mask to MASK.
for (int k = 0; k < MASK; k++) {
TestPeeledReductionNode.mask++;
}
for (int i = 0; i < N; i++) {
for (int j = 0; j < M; j++) {
// Before the fix, this reduction is peeled out of its loop and
// wrongly remains marked as a reduction within the outer loop.
r[i] ^= TestPeeledReductionNode.mask;
}
}
for (int i = 0; i < N; i++) {
Asserts.assertEquals(r[i], EXPECTED);
}
return;
}
}

3 comments on commit 47e478d

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GoeLin
Copy link
Member Author

@GoeLin GoeLin commented on 47e478d Jul 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk11u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 47e478d Jul 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GoeLin the backport was successfully created on the branch GoeLin-backport-47e478d6 in my personal fork of openjdk/jdk11u-dev. To create a pull request with this backport targeting openjdk/jdk11u-dev:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit 47e478d6 from the openjdk/jdk17u-dev repository.

The commit being backported was authored by Goetz Lindenmaier on 24 Jun 2022 and had no reviewers.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk11u-dev:

$ git fetch https://github.com/openjdk-bots/jdk11u-dev GoeLin-backport-47e478d6:GoeLin-backport-47e478d6
$ git checkout GoeLin-backport-47e478d6
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk11u-dev GoeLin-backport-47e478d6

Please sign in to comment.