Skip to content

Commit

Permalink
8299179: ArrayFill with store on backedge needs to reduce length by 1
Browse files Browse the repository at this point in the history
Reviewed-by: thartmann, kvn
  • Loading branch information
eme64 committed Jan 12, 2023
1 parent af8d3fb commit d716ec5
Show file tree
Hide file tree
Showing 3 changed files with 429 additions and 0 deletions.
20 changes: 20 additions & 0 deletions src/hotspot/share/opto/loopTransform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4381,6 +4381,19 @@ bool PhaseIdealLoop::intrinsify_fill(IdealLoopTree* lpt) {
Node* len = new SubINode(head->limit(), head->init_trip());
_igvn.register_new_node_with_optimizer(len);

// If the store is on the backedge, it is not executed in the last
// iteration, and we must subtract 1 from the len.
Node* backedge = head->loopexit()->proj_out(1);
if (store->in(0) == backedge) {
len = new SubINode(len, _igvn.intcon(1));
_igvn.register_new_node_with_optimizer(len);
#ifndef PRODUCT
if (TraceOptimizeFill) {
tty->print_cr("ArrayFill store on backedge, subtract 1 from len.");
}
#endif
}

BasicType t = store->as_Mem()->memory_type();
bool aligned = false;
if (offset != NULL && head->init_trip()->is_Con()) {
Expand Down Expand Up @@ -4483,5 +4496,12 @@ bool PhaseIdealLoop::intrinsify_fill(IdealLoopTree* lpt) {
_igvn.replace_node(n, C->top());
}

#ifndef PRODUCT
if (TraceOptimizeFill) {
tty->print("ArrayFill call ");
call->dump();
}
#endif

return true;
}
178 changes: 178 additions & 0 deletions test/hotspot/jtreg/compiler/loopopts/TestBackedgeLoadArrayFill.jasm
Original file line number Diff line number Diff line change
@@ -0,0 +1,178 @@
/*
* Copyright (c) 2023, 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.
*
*/

super public class TestBackedgeLoadArrayFill
{
public Method "<init>":"()V"
stack 2 locals 1
{
aload_0;
invokespecial Method java/lang/Object."<init>":"()V";
return;
}

static Method test_101:"()V"
stack 20 locals 20
{
// test_002 in jasm: using try-catch
ldc 6;
istore_0; // i = 6
ldc 25;
newarray int;
astore_1; // arr = new int[25]
HEAD:
aload_1;
iload_0;
iconst_1;
iastore; // arr[i] = 1
// second block - the only one -> head block can be copied: one before, one on backedge
try t0;
aload_1;
iload_0;
aload_1;
iload_0;
iaload;
iastore; // arr[i] = arr[i]
goto FINALLY;
endtry t0;
catch t0 java/lang/Exception;
pop; // exception
FINALLY:
iinc 0, 1; // i++
iload_0;
ldc 21;
if_icmplt HEAD; // if i < 21
// write array
aload_1;
putstatic Field TestBackedgeLoadArrayFillMain.intA:"[I";
return;
}

static Method test_102:"()V"
stack 20 locals 20
{
// test_002 in jasm: without try-catch
ldc 5;
istore_0; // i = 5
ldc 25;
newarray int;
astore_1; // arr = new int[25]
HEAD:
aload_1;
iload_0;
iconst_1;
iastore; // arr[i] = 1
goto SECOND;
// second block - the only one -> head block can be copied: one before, one on backedge
// must have some material before inc, else it is partial peeled away
// And if we set -XX:-PartialPeelLoop, then the counted loop is never detected
SECOND:
aload_1;
iload_0;
aload_1;
iload_0;
iaload;
iastore; // arr[i] = arr[i]
iinc 0, 1; // i++
iload_0;
ldc 21;
if_icmplt HEAD; // if i < 21
// write array
aload_1;
putstatic Field TestBackedgeLoadArrayFillMain.intA:"[I";
return;
}

static Method test_103:"()V"
stack 20 locals 20
{
// test_002 in jasm: without try-catch, and second array
ldc 7;
istore_0; // i = 7
ldc 25;
newarray int;
astore_1; // arr = new int[25]
ldc 25;
newarray int;
astore_2; // arr2 = new int[25]
HEAD:
aload_1;
iload_0;
iconst_1;
iastore; // arr[i] = 1
goto SECOND;
// second block - the only one -> head block can be copied: one before, one on backedge
SECOND:
// we can also do the identity read-write on another array - it just has to eventually disappear
aload_2;
iload_0;
aload_2;
iload_0;
iaload;
iastore; // arr2[i] = arr2[i]

iinc 0, 1; // i++
iload_0;
ldc 21;
if_icmplt HEAD; // if i < 21
// write array
aload_1;
putstatic Field TestBackedgeLoadArrayFillMain.intA:"[I";
return;
}

static Method test_104:"()V"
stack 20 locals 20
{
ldc 9;
istore_0; // i = 9
ldc 25;
newarray int;
astore_1; // arr = new int[25]
HEAD:
aload_1;
iload_0;
iconst_1;
iastore; // arr[i] = 1
goto SECOND;
// second block - the only one -> head block can be copied: one before, one on backedge
SECOND:
// CFG leads to partial peel -> load moved into loop body, then intrinsified
iload_0;
ldc 2;
irem;
ifeq SKIP;

SKIP:

iinc 0, 1; // i++
iload_0;
ldc 21;
if_icmplt HEAD; // if i < 21
// write array
aload_1;
putstatic Field TestBackedgeLoadArrayFillMain.intA:"[I";
return;
}
}

3 comments on commit d716ec5

@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

@GoeLin GoeLin commented on d716ec5 Apr 19, 2023

Choose a reason for hiding this comment

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

/backport jdk17u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on d716ec5 Apr 19, 2023

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-d716ec5d in my personal fork of openjdk/jdk17u-dev. To create a pull request with this backport targeting openjdk/jdk17u-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 d716ec5d from the openjdk/jdk repository.

The commit being backported was authored by Emanuel Peter on 12 Jan 2023 and was reviewed by Tobias Hartmann and Vladimir Kozlov.

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/jdk17u-dev:

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

Please sign in to comment.