Skip to content

Revert "[GVNSink] Fix incorrect codegen with respect to GEPs #85333" #90658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

hiraditya
Copy link
Collaborator

Reverts #88440

Test failing on Windows: https://lab.llvm.org/buildbot/#/builders/233/builds/9396

Input file: <stdin>
# | Check file: C:\buildbot\as-builder-8\llvm-nvptx-nvidia-win\llvm-project\llvm\test\Transforms\GVNSink\different-gep-types.ll
# | 
# | -dump-input=help explains the following input dump.
# | 
# | Input was:
# | <<<<<<
# |            .
# |            .
# |            .
# |           42:  br label %if.end6 
# |           43:  
# |           44: if.else5: ; preds = %if.else 
# |           45:  br label %if.end6 
# |           46:  
# |           47: if.end6: ; preds = %if.else5, %if.then3, %if.then 
# | next:67'0             X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
# | next:67'1                                                        with "IF_THEN" equal to "%if\\.then"
# | next:67'2                                                        with "IF_THEN3" equal to "%if\\.then3"
# | next:67'3                                                        with "IF_ELSE5" equal to "%if\\.else5"
# |           48:  %.sink1 = phi i32 [ -8, %if.then3 ], [ -4, %if.else5 ], [ 8, %if.then ] 
# | next:67'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# | next:67'4      ?                                                                        possible intended match
# |           49:  %0 = load ptr, ptr %__i, align 4 
# | next:67'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# |           50:  %incdec.ptr4 = getelementptr inbounds i8, ptr %0, i32 %.sink1 
# | next:67'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# |           51:  store ptr %incdec.ptr4, ptr %__i, align 4 
# | next:67'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# |           52:  ret void 
# | next:67'0     ~~~~~~~~~~
# |           53: } 
# | next:67'0     ~~
# | >>>>>>
# `-----------------------------
# error: command failed with exit status: 1

@llvmbot
Copy link
Member

llvmbot commented Apr 30, 2024

@llvm/pr-subscribers-llvm-transforms

Author: AdityaK (hiraditya)

Changes

Reverts llvm/llvm-project#88440

Test failing on Windows: https://lab.llvm.org/buildbot/#/builders/233/builds/9396

Input file: &lt;stdin&gt;
# | Check file: C:\buildbot\as-builder-8\llvm-nvptx-nvidia-win\llvm-project\llvm\test\Transforms\GVNSink\different-gep-types.ll
# | 
# | -dump-input=help explains the following input dump.
# | 
# | Input was:
# | &lt;&lt;&lt;&lt;&lt;&lt;
# |            .
# |            .
# |            .
# |           42:  br label %if.end6 
# |           43:  
# |           44: if.else5: ; preds = %if.else 
# |           45:  br label %if.end6 
# |           46:  
# |           47: if.end6: ; preds = %if.else5, %if.then3, %if.then 
# | next:67'0             X~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ error: no match found
# | next:67'1                                                        with "IF_THEN" equal to "%if\\.then"
# | next:67'2                                                        with "IF_THEN3" equal to "%if\\.then3"
# | next:67'3                                                        with "IF_ELSE5" equal to "%if\\.else5"
# |           48:  %.sink1 = phi i32 [ -8, %if.then3 ], [ -4, %if.else5 ], [ 8, %if.then ] 
# | next:67'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# | next:67'4      ?                                                                        possible intended match
# |           49:  %0 = load ptr, ptr %__i, align 4 
# | next:67'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# |           50:  %incdec.ptr4 = getelementptr inbounds i8, ptr %0, i32 %.sink1 
# | next:67'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# |           51:  store ptr %incdec.ptr4, ptr %__i, align 4 
# | next:67'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# |           52:  ret void 
# | next:67'0     ~~~~~~~~~~
# |           53: } 
# | next:67'0     ~~
# | &gt;&gt;&gt;&gt;&gt;&gt;
# `-----------------------------
# error: command failed with exit status: 1

Full diff: https://github.com/llvm/llvm-project/pull/90658.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/GVNSink.cpp (+5-4)
  • (removed) llvm/test/Transforms/GVNSink/different-gep-types.ll (-101)
diff --git a/llvm/lib/Transforms/Scalar/GVNSink.cpp b/llvm/lib/Transforms/Scalar/GVNSink.cpp
index 1d2577e1da3375..d4907326eb0a5a 100644
--- a/llvm/lib/Transforms/Scalar/GVNSink.cpp
+++ b/llvm/lib/Transforms/Scalar/GVNSink.cpp
@@ -719,11 +719,12 @@ GVNSink::analyzeInstructionForSinking(LockstepReverseIterator &LRI,
   // try and continue making progress.
   Instruction *I0 = NewInsts[0];
 
-  auto isNotSameOperation = [&I0](Instruction *I) {
-    return !I0->isSameOperationAs(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.
+  auto hasDifferentNumOperands = [&I0](Instruction *I) {
+    return I->getNumOperands() != I0->getNumOperands();
   };
-
-  if (any_of(NewInsts, isNotSameOperation))
+  if (any_of(NewInsts, hasDifferentNumOperands))
     return std::nullopt;
 
   for (unsigned OpNum = 0, E = I0->getNumOperands(); OpNum != E; ++OpNum) {
diff --git a/llvm/test/Transforms/GVNSink/different-gep-types.ll b/llvm/test/Transforms/GVNSink/different-gep-types.ll
deleted file mode 100644
index 77cdc8a94f97c2..00000000000000
--- a/llvm/test/Transforms/GVNSink/different-gep-types.ll
+++ /dev/null
@@ -1,101 +0,0 @@
-; 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"
-
-%"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
-}

@hiraditya hiraditya merged commit cf49d07 into main Apr 30, 2024
5 of 6 checks passed
@hiraditya hiraditya deleted the revert-88440-gvnsink branch April 30, 2024 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants