[llvm] Fix incorrect codegen with respect to GEPs #85333 (PR #88440)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 26 15:03:50 PDT 2024
https://github.com/hiraditya updated https://github.com/llvm/llvm-project/pull/88440
>From 6f13d795dba93f685b61447e8072b6a70821e49b Mon Sep 17 00:00:00 2001
From: AdityaK <1894981+hiraditya at users.noreply.github.com>
Date: Thu, 11 Apr 2024 14:04:45 -0700
Subject: [PATCH] Fix incorrect codegen with respect to GEPs #85333
---
llvm/lib/Transforms/Scalar/GVNSink.cpp | 27 +++--
.../Transforms/GVNSink/different-gep-types.ll | 102 ++++++++++++++++++
2 files changed, 121 insertions(+), 8 deletions(-)
create mode 100644 llvm/test/Transforms/GVNSink/different-gep-types.ll
diff --git a/llvm/lib/Transforms/Scalar/GVNSink.cpp b/llvm/lib/Transforms/Scalar/GVNSink.cpp
index d4907326eb0a5a..a64ae46d2a72a3 100644
--- a/llvm/lib/Transforms/Scalar/GVNSink.cpp
+++ b/llvm/lib/Transforms/Scalar/GVNSink.cpp
@@ -719,21 +719,32 @@ GVNSink::analyzeInstructionForSinking(LockstepReverseIterator &LRI,
// try and continue making progress.
Instruction *I0 = NewInsts[0];
- // If all instructions that are going to participate don't have the same
- // number of operands, we can't do any useful PHI analysis for all operands.
- auto hasDifferentNumOperands = [&I0](Instruction *I) {
- return I->getNumOperands() != I0->getNumOperands();
+ auto hasDifferentOperands = [&I0](Instruction *I) {
+ // If all instructions that are going to participate don't have the same
+ // number of operands, we can't do any useful PHI analysis for all operands.
+ if (I->getNumOperands() != I0->getNumOperands())
+ return true;
+ // Having different source element types may result in incorrect
+ // pointer arithmetic on geps(github.com/llvm/llvm-project/issues/85333)
+ if (auto *II = dyn_cast<GetElementPtrInst>(I)) {
+ auto I0I = cast<GetElementPtrInst>(I0);
+ return II->getSourceElementType() != I0I->getSourceElementType();
+ }
+ return false;
};
- if (any_of(NewInsts, hasDifferentNumOperands))
+
+ if (any_of(NewInsts, hasDifferentOperands))
return std::nullopt;
for (unsigned OpNum = 0, E = I0->getNumOperands(); OpNum != E; ++OpNum) {
ModelledPHI PHI(NewInsts, OpNum, ActivePreds);
if (PHI.areAllIncomingValuesSame())
continue;
- if (!canReplaceOperandWithVariable(I0, OpNum))
- // We can 't create a PHI from this instruction!
- return std::nullopt;
+ for (auto &Candidate : NewInsts) {
+ if (!canReplaceOperandWithVariable(Candidate, OpNum))
+ // We can 't create a PHI from this instruction!
+ return std::nullopt;
+ }
if (NeededPHIs.count(PHI))
continue;
if (!PHI.areAllIncomingValuesSameType())
diff --git a/llvm/test/Transforms/GVNSink/different-gep-types.ll b/llvm/test/Transforms/GVNSink/different-gep-types.ll
new file mode 100644
index 00000000000000..d7d948b6c4cc6e
--- /dev/null
+++ b/llvm/test/Transforms/GVNSink/different-gep-types.ll
@@ -0,0 +1,102 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
+; RUN: opt -passes=gvn-sink -S %s | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64"
+target triple = "thumbv7em-none-unknown-eabi"
+
+%"struct.std::pair" = type <{ i32, %struct.substruct, [2 x i8] }>
+%struct.substruct = type { i8, i8 }
+%"struct.std::random_access_iterator_tag" = type { i8 }
+
+; Check that gep is not sunk as they are of different types.
+define void @bar(ptr noundef nonnull align 4 dereferenceable(4) %__i, i32 noundef %__n) {
+; CHECK-LABEL: define void @bar(
+; CHECK-SAME: ptr noundef nonnull align 4 dereferenceable(4) [[__I:%.*]], i32 noundef [[__N:%.*]]) {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[__N]], 1
+; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
+; CHECK: if.then:
+; CHECK-NEXT: [[TMP0:%.*]] = load ptr, ptr [[__I]], align 4
+; CHECK-NEXT: [[INCDEC_PTR4:%.*]] = getelementptr inbounds i8, ptr [[TMP0]], i32 -8
+; CHECK-NEXT: br label [[IF_END6:%.*]]
+; CHECK: if.else:
+; CHECK-NEXT: [[TMP1:%.*]] = load ptr, ptr [[__I]], align 4
+; CHECK-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds %"struct.std::pair", ptr [[TMP1]], i32 [[__N]]
+; CHECK-NEXT: br label [[IF_END6]]
+; CHECK: if.end6:
+; CHECK-NEXT: [[INCDEC_PTR_SINK:%.*]] = phi ptr [ [[INCDEC_PTR4]], [[IF_THEN]] ], [ [[ADD_PTR]], [[IF_ELSE]] ]
+; CHECK-NEXT: store ptr [[INCDEC_PTR_SINK]], ptr [[__I]], align 4
+; CHECK-NEXT: ret void
+;
+entry:
+ %cmp = icmp eq i32 %__n, 1
+ br i1 %cmp, label %if.then, label %if.else
+
+if.then:
+ %3 = load ptr, ptr %__i, align 4
+ %incdec.ptr4 = getelementptr inbounds i8, ptr %3, i32 -8
+ br label %if.end6
+
+if.else:
+ %4 = load ptr, ptr %__i, align 4
+ %add.ptr = getelementptr inbounds %"struct.std::pair", ptr %4, i32 %__n
+ br label %if.end6
+
+if.end6:
+ %incdec.ptr.sink = phi ptr [ %incdec.ptr4, %if.then ], [ %add.ptr, %if.else ]
+ store ptr %incdec.ptr.sink, ptr %__i, align 4
+ ret void
+}
+
+; Check that load,gep, and store are all sunk as they are safe to do.
+define void @foo(ptr noundef nonnull align 4 dereferenceable(4) %__i, i32 noundef %__n) {
+; CHECK-LABEL: define void @foo(
+; CHECK-SAME: ptr noundef nonnull align 4 dereferenceable(4) [[__I:%.*]], i32 noundef [[__N:%.*]]) {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[__N]], 1
+; CHECK-NEXT: br i1 [[CMP]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
+; CHECK: if.then:
+; CHECK-NEXT: br label [[IF_END6:%.*]]
+; CHECK: if.else:
+; CHECK-NEXT: [[CMP2:%.*]] = icmp eq i32 [[__N]], -1
+; CHECK-NEXT: br i1 [[CMP2]], label [[IF_THEN3:%.*]], label [[IF_ELSE5:%.*]]
+; CHECK: if.then3:
+; CHECK-NEXT: br label [[IF_END6]]
+; CHECK: if.else5:
+; CHECK-NEXT: br label [[IF_END6]]
+; CHECK: if.end6:
+; CHECK-NEXT: [[DOTSINK1:%.*]] = phi i32 [ 8, [[IF_THEN]] ], [ -8, [[IF_THEN3]] ], [ -4, [[IF_ELSE5]] ]
+; CHECK-NEXT: [[TMP0:%.*]] = load ptr, ptr [[__I]], align 4
+; CHECK-NEXT: [[INCDEC_PTR:%.*]] = getelementptr inbounds i8, ptr [[TMP0]], i32 [[DOTSINK1]]
+; CHECK-NEXT: store ptr [[INCDEC_PTR]], ptr [[__I]], align 4
+; CHECK-NEXT: ret void
+;
+entry:
+ %cmp = icmp eq i32 %__n, 1
+ br i1 %cmp, label %if.then, label %if.else
+
+if.then:
+ %1 = load ptr, ptr %__i, align 4
+ %incdec.ptr = getelementptr inbounds i8, ptr %1, i32 8
+ store ptr %incdec.ptr, ptr %__i, align 4
+ br label %if.end6
+
+if.else:
+ %cmp2 = icmp eq i32 %__n, -1
+ br i1 %cmp2, label %if.then3, label %if.else5
+
+if.then3:
+ %3 = load ptr, ptr %__i, align 4
+ %incdec.ptr4 = getelementptr inbounds i8, ptr %3, i32 -8
+ store ptr %incdec.ptr4, ptr %__i, align 4
+ br label %if.end6
+
+if.else5:
+ %4 = load ptr, ptr %__i, align 4
+ %add.ptr = getelementptr inbounds i8, ptr %4, i32 -4
+ store ptr %add.ptr, ptr %__i, align 4
+ br label %if.end6
+
+if.end6:
+ ret void
+}
More information about the llvm-commits
mailing list