[llvm-branch-commits] [llvm] release/18.x: [ArgPromotion] Remove incorrect TranspBlocks set for loads. (#84835) (PR #84945)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Wed Mar 13 11:01:47 PDT 2024
https://github.com/llvmbot updated https://github.com/llvm/llvm-project/pull/84945
>From 25a989ce8bf35ccda064d956305f920bf711a7de Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Mon, 11 Mar 2024 21:06:03 +0000
Subject: [PATCH 1/2] [ArgPromotion] Add test case for #84807.
Test case for https://github.com/llvm/llvm-project/issues/84807,
showing a mis-compile in ArgPromotion.
(cherry picked from commit 31ffdb56b4df9b772d763dccabbfde542545d695)
---
...ing-and-non-aliasing-loads-with-clobber.ll | 100 ++++++++++++++++++
1 file changed, 100 insertions(+)
create mode 100644 llvm/test/Transforms/ArgumentPromotion/aliasing-and-non-aliasing-loads-with-clobber.ll
diff --git a/llvm/test/Transforms/ArgumentPromotion/aliasing-and-non-aliasing-loads-with-clobber.ll b/llvm/test/Transforms/ArgumentPromotion/aliasing-and-non-aliasing-loads-with-clobber.ll
new file mode 100644
index 00000000000000..69385a7ea51a74
--- /dev/null
+++ b/llvm/test/Transforms/ArgumentPromotion/aliasing-and-non-aliasing-loads-with-clobber.ll
@@ -0,0 +1,100 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; RUN: opt -p argpromotion -S %s | FileCheck %s
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"
+
+ at f = dso_local global { i16, i64 } { i16 1, i64 0 }, align 8
+
+; Test case for https://github.com/llvm/llvm-project/issues/84807.
+
+; FIXME: Currently the loads from @callee are moved to @caller, even though
+; the store in %then may aliases to load from %q.
+
+define i32 @caller1(i1 %c) {
+; CHECK-LABEL: define i32 @caller1(
+; CHECK-SAME: i1 [[C:%.*]]) {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[F_VAL:%.*]] = load i16, ptr @f, align 8
+; CHECK-NEXT: [[TMP0:%.*]] = getelementptr i8, ptr @f, i64 8
+; CHECK-NEXT: [[F_VAL1:%.*]] = load i64, ptr [[TMP0]], align 8
+; CHECK-NEXT: call void @callee1(i16 [[F_VAL]], i64 [[F_VAL1]], i1 [[C]])
+; CHECK-NEXT: ret i32 0
+;
+entry:
+ call void @callee1(ptr noundef nonnull @f, i1 %c)
+ ret i32 0
+}
+
+define internal void @callee1(ptr nocapture noundef readonly %q, i1 %c) {
+; CHECK-LABEL: define internal void @callee1(
+; CHECK-SAME: i16 [[Q_0_VAL:%.*]], i64 [[Q_8_VAL:%.*]], i1 [[C:%.*]]) {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br i1 [[C]], label [[THEN:%.*]], label [[EXIT:%.*]]
+; CHECK: then:
+; CHECK-NEXT: store i16 123, ptr @f, align 8
+; CHECK-NEXT: br label [[EXIT]]
+; CHECK: exit:
+; CHECK-NEXT: call void @use(i16 [[Q_0_VAL]], i64 [[Q_8_VAL]])
+; CHECK-NEXT: ret void
+;
+entry:
+ br i1 %c, label %then, label %exit
+
+then:
+ store i16 123, ptr @f, align 8
+ br label %exit
+
+exit:
+ %l.0 = load i16, ptr %q, align 8
+ %gep.8 = getelementptr inbounds i8, ptr %q, i64 8
+ %l.1 = load i64, ptr %gep.8, align 8
+ call void @use(i16 %l.0, i64 %l.1)
+ ret void
+
+ uselistorder ptr %q, { 1, 0 }
+}
+
+; Same as @caller1/callee2, but with default uselist order.
+define i32 @caller2(i1 %c) {
+; CHECK-LABEL: define i32 @caller2(
+; CHECK-SAME: i1 [[C:%.*]]) {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: call void @callee2(ptr noundef nonnull @f, i1 [[C]])
+; CHECK-NEXT: ret i32 0
+;
+entry:
+ call void @callee2(ptr noundef nonnull @f, i1 %c)
+ ret i32 0
+}
+
+define internal void @callee2(ptr nocapture noundef readonly %q, i1 %c) {
+; CHECK-LABEL: define internal void @callee2(
+; CHECK-SAME: ptr nocapture noundef readonly [[Q:%.*]], i1 [[C:%.*]]) {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br i1 [[C]], label [[THEN:%.*]], label [[EXIT:%.*]]
+; CHECK: then:
+; CHECK-NEXT: store i16 123, ptr @f, align 8
+; CHECK-NEXT: br label [[EXIT]]
+; CHECK: exit:
+; CHECK-NEXT: [[Q_0_VAL:%.*]] = load i16, ptr [[Q]], align 8
+; CHECK-NEXT: [[GEP_8:%.*]] = getelementptr inbounds i8, ptr [[Q]], i64 8
+; CHECK-NEXT: [[Q_8_VAL:%.*]] = load i64, ptr [[GEP_8]], align 8
+; CHECK-NEXT: call void @use(i16 [[Q_0_VAL]], i64 [[Q_8_VAL]])
+; CHECK-NEXT: ret void
+;
+entry:
+ br i1 %c, label %then, label %exit
+
+then:
+ store i16 123, ptr @f, align 8
+ br label %exit
+
+exit:
+ %l.0 = load i16, ptr %q, align 8
+ %gep.8 = getelementptr inbounds i8, ptr %q, i64 8
+ %l.1 = load i64, ptr %gep.8, align 8
+ call void @use(i16 %l.0, i64 %l.1)
+ ret void
+}
+
+declare void @use(i16, i64)
>From 7b61ddefc28a2c88be3a754ceee7bace98e3b187 Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Tue, 12 Mar 2024 09:47:42 +0000
Subject: [PATCH 2/2] [ArgPromotion] Remove incorrect TranspBlocks set for
loads. (#84835)
The TranspBlocks set was used to cache aliasing decision for all
processed loads in the parent loop. This is incorrect, because each load
can access a different location, which means one load not being modified
in a block doesn't translate to another load not being modified in the
same block.
All loads access the same underlying object, so we could perhaps use a
location without size for all loads and retain the cache, but that would
mean we loose precision.
For now, just drop the cache.
Fixes https://github.com/llvm/llvm-project/issues/84807
PR: https://github.com/llvm/llvm-project/pull/84835
(cherry picked from commit bba4a1daff6ee09941f1369a4e56b4af95efdc5c)
---
llvm/lib/Transforms/IPO/ArgumentPromotion.cpp | 6 +-----
...aliasing-and-non-aliasing-loads-with-clobber.ll | 14 +++++++-------
2 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
index 8058282c422503..062a3d341007ce 100644
--- a/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
+++ b/llvm/lib/Transforms/IPO/ArgumentPromotion.cpp
@@ -652,10 +652,6 @@ static bool findArgParts(Argument *Arg, const DataLayout &DL, AAResults &AAR,
// check to see if the pointer is guaranteed to not be modified from entry of
// the function to each of the load instructions.
- // Because there could be several/many load instructions, remember which
- // blocks we know to be transparent to the load.
- df_iterator_default_set<BasicBlock *, 16> TranspBlocks;
-
for (LoadInst *Load : Loads) {
// Check to see if the load is invalidated from the start of the block to
// the load itself.
@@ -669,7 +665,7 @@ static bool findArgParts(Argument *Arg, const DataLayout &DL, AAResults &AAR,
// To do this, we perform a depth first search on the inverse CFG from the
// loading block.
for (BasicBlock *P : predecessors(BB)) {
- for (BasicBlock *TranspBB : inverse_depth_first_ext(P, TranspBlocks))
+ for (BasicBlock *TranspBB : inverse_depth_first(P))
if (AAR.canBasicBlockModify(*TranspBB, Loc))
return false;
}
diff --git a/llvm/test/Transforms/ArgumentPromotion/aliasing-and-non-aliasing-loads-with-clobber.ll b/llvm/test/Transforms/ArgumentPromotion/aliasing-and-non-aliasing-loads-with-clobber.ll
index 69385a7ea51a74..1e1669b29b0db6 100644
--- a/llvm/test/Transforms/ArgumentPromotion/aliasing-and-non-aliasing-loads-with-clobber.ll
+++ b/llvm/test/Transforms/ArgumentPromotion/aliasing-and-non-aliasing-loads-with-clobber.ll
@@ -7,17 +7,14 @@ target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:
; Test case for https://github.com/llvm/llvm-project/issues/84807.
-; FIXME: Currently the loads from @callee are moved to @caller, even though
-; the store in %then may aliases to load from %q.
+; Make sure the loads from @callee are not moved to @caller, as the store
+; in %then may aliases to load from %q.
define i32 @caller1(i1 %c) {
; CHECK-LABEL: define i32 @caller1(
; CHECK-SAME: i1 [[C:%.*]]) {
; CHECK-NEXT: entry:
-; CHECK-NEXT: [[F_VAL:%.*]] = load i16, ptr @f, align 8
-; CHECK-NEXT: [[TMP0:%.*]] = getelementptr i8, ptr @f, i64 8
-; CHECK-NEXT: [[F_VAL1:%.*]] = load i64, ptr [[TMP0]], align 8
-; CHECK-NEXT: call void @callee1(i16 [[F_VAL]], i64 [[F_VAL1]], i1 [[C]])
+; CHECK-NEXT: call void @callee1(ptr noundef nonnull @f, i1 [[C]])
; CHECK-NEXT: ret i32 0
;
entry:
@@ -27,13 +24,16 @@ entry:
define internal void @callee1(ptr nocapture noundef readonly %q, i1 %c) {
; CHECK-LABEL: define internal void @callee1(
-; CHECK-SAME: i16 [[Q_0_VAL:%.*]], i64 [[Q_8_VAL:%.*]], i1 [[C:%.*]]) {
+; CHECK-SAME: ptr nocapture noundef readonly [[Q:%.*]], i1 [[C:%.*]]) {
; CHECK-NEXT: entry:
; CHECK-NEXT: br i1 [[C]], label [[THEN:%.*]], label [[EXIT:%.*]]
; CHECK: then:
; CHECK-NEXT: store i16 123, ptr @f, align 8
; CHECK-NEXT: br label [[EXIT]]
; CHECK: exit:
+; CHECK-NEXT: [[Q_0_VAL:%.*]] = load i16, ptr [[Q]], align 8
+; CHECK-NEXT: [[GEP_8:%.*]] = getelementptr inbounds i8, ptr [[Q]], i64 8
+; CHECK-NEXT: [[Q_8_VAL:%.*]] = load i64, ptr [[GEP_8]], align 8
; CHECK-NEXT: call void @use(i16 [[Q_0_VAL]], i64 [[Q_8_VAL]])
; CHECK-NEXT: ret void
;
More information about the llvm-branch-commits
mailing list