[llvm] 831e99f - [GVNHoist] don't hoist callbr users into the callbr's block

Nick Desaulniers via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 13 10:21:13 PDT 2023


Author: Nick Desaulniers
Date: 2023-03-13T10:20:57-07:00
New Revision: 831e99fee90e9406c7260ca66d688ec353110183

URL: https://github.com/llvm/llvm-project/commit/831e99fee90e9406c7260ca66d688ec353110183
DIFF: https://github.com/llvm/llvm-project/commit/831e99fee90e9406c7260ca66d688ec353110183.diff

LOG: [GVNHoist] don't hoist callbr users into the callbr's block

This isn't safe to do.

Link: https://github.com/llvm/llvm-project/issues/53562
Fixes: https://github.com/llvm/llvm-project/issues/61023

Reviewed By: efriedma, nikic

Differential Revision: https://reviews.llvm.org/D144927

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/GVNHoist.cpp
    llvm/test/Transforms/GVNHoist/hoist-call.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/GVNHoist.cpp b/llvm/lib/Transforms/Scalar/GVNHoist.cpp
index bbff497b7d92d..f13b52fecba3d 100644
--- a/llvm/lib/Transforms/Scalar/GVNHoist.cpp
+++ b/llvm/lib/Transforms/Scalar/GVNHoist.cpp
@@ -808,15 +808,20 @@ bool GVNHoist::valueAnticipable(CHIArgs C, Instruction *TI) const {
 void GVNHoist::checkSafety(CHIArgs C, BasicBlock *BB, GVNHoist::InsKind K,
                            SmallVectorImpl<CHIArg> &Safe) {
   int NumBBsOnAllPaths = MaxNumberOfBBSInPath;
+  const Instruction *T = BB->getTerminator();
   for (auto CHI : C) {
     Instruction *Insn = CHI.I;
     if (!Insn) // No instruction was inserted in this CHI.
       continue;
+    // If the Terminator is some kind of "exotic terminator" that produces a
+    // value (such as InvokeInst, CallBrInst, or CatchSwitchInst) which the CHI
+    // uses, it is not safe to hoist the use above the def.
+    if (!T->use_empty() && is_contained(Insn->operands(), T))
+      continue;
     if (K == InsKind::Scalar) {
       if (safeToHoistScalar(BB, Insn->getParent(), NumBBsOnAllPaths))
         Safe.push_back(CHI);
     } else {
-      auto *T = BB->getTerminator();
       if (MemoryUseOrDef *UD = MSSA->getMemoryAccess(Insn))
         if (safeToHoistLdSt(T, Insn, UD, K, NumBBsOnAllPaths))
           Safe.push_back(CHI);

diff  --git a/llvm/test/Transforms/GVNHoist/hoist-call.ll b/llvm/test/Transforms/GVNHoist/hoist-call.ll
index 206b782567854..7c1fe26a827cc 100644
--- a/llvm/test/Transforms/GVNHoist/hoist-call.ll
+++ b/llvm/test/Transforms/GVNHoist/hoist-call.ll
@@ -1,13 +1,20 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -S -passes=gvn-hoist < %s | FileCheck %s
 
 ; Check that the call and fcmp are hoisted.
-; CHECK-LABEL: define void @fun(
-; CHECK: call float
-; CHECK: fcmp oeq
-; CHECK-NOT: call float
-; CHECK-NOT: fcmp oeq
-
 define void @fun(float %__b) minsize {
+; CHECK-LABEL: @fun(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[IF_THEN:%.*]]
+; CHECK:       if.then:
+; CHECK-NEXT:    [[TMP0:%.*]] = call float @llvm.fabs.f32(float [[__B:%.*]])
+; CHECK-NEXT:    [[CMPINF7:%.*]] = fcmp oeq float [[TMP0]], 0x7FF0000000000000
+; CHECK-NEXT:    br i1 undef, label [[IF_THEN8:%.*]], label [[LOR_LHS_FALSE:%.*]]
+; CHECK:       lor.lhs.false:
+; CHECK-NEXT:    unreachable
+; CHECK:       if.then8:
+; CHECK-NEXT:    ret void
+;
 entry:
   br label %if.then
 
@@ -26,3 +33,171 @@ if.then8:                                         ; preds = %if.then
 }
 
 declare float @llvm.fabs.f32(float)
+
+; Check that extractvalues are not hoisted into entry, but that non-dependent
+; adds are.
+define i32 @foo(i32 %x) {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[ADD:%.*]] = add nsw i32 [[X:%.*]], 1
+; CHECK-NEXT:    [[TMP0:%.*]] = callbr { i32, i32 } asm sideeffect "somestuff", "=r,=r,!i"()
+; CHECK-NEXT:    to label [[ASM_FALLTHROUGH:%.*]] [label %err.split]
+; CHECK:       asm.fallthrough:
+; CHECK-NEXT:    [[ASMRESULT:%.*]] = extractvalue { i32, i32 } [[TMP0]], 0
+; CHECK-NEXT:    ret i32 [[ADD]]
+; CHECK:       err.split:
+; CHECK-NEXT:    [[ASMRESULT2:%.*]] = extractvalue { i32, i32 } [[TMP0]], 0
+; CHECK-NEXT:    ret i32 [[ADD]]
+;
+entry:
+  %0 = callbr { i32, i32 } asm sideeffect "somestuff", "=r,=r,!i"()
+  to label %asm.fallthrough [label %err.split]
+
+asm.fallthrough:                                  ; preds = %entry
+  %asmresult = extractvalue { i32, i32 } %0, 0
+  %add = add nsw i32 %x, 1
+  ret i32 %add
+
+err.split:                                        ; preds = %entry
+  %asmresult2 = extractvalue { i32, i32 } %0, 0
+  %add2 = add nsw i32 %x, 1
+  ret i32 %add2
+}
+
+; Check that extractvalues and dependent adds are not hoisted into entry.
+define i32 @foo2() {
+; CHECK-LABEL: @foo2(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TMP0:%.*]] = callbr { i32, i32 } asm sideeffect "somestuff", "=r,=r,!i"()
+; CHECK-NEXT:    to label [[ASM_FALLTHROUGH:%.*]] [label %err.split]
+; CHECK:       asm.fallthrough:
+; CHECK-NEXT:    [[ASMRESULT:%.*]] = extractvalue { i32, i32 } [[TMP0]], 0
+; CHECK-NEXT:    [[ADD:%.*]] = add nsw i32 [[ASMRESULT]], 1
+; CHECK-NEXT:    ret i32 [[ADD]]
+; CHECK:       err.split:
+; CHECK-NEXT:    [[ASMRESULT2:%.*]] = extractvalue { i32, i32 } [[TMP0]], 0
+; CHECK-NEXT:    [[ADD2:%.*]] = add nsw i32 [[ASMRESULT2]], 1
+; CHECK-NEXT:    ret i32 [[ADD2]]
+;
+entry:
+  %0 = callbr { i32, i32 } asm sideeffect "somestuff", "=r,=r,!i"()
+  to label %asm.fallthrough [label %err.split]
+
+asm.fallthrough:                                  ; preds = %entry
+  %asmresult = extractvalue { i32, i32 } %0, 0
+  %add = add nsw i32 %asmresult, 1
+  ret i32 %add
+
+err.split:                                        ; preds = %entry
+  %asmresult2 = extractvalue { i32, i32 } %0, 0
+  %add2 = add nsw i32 %asmresult2, 1
+  ret i32 %add2
+}
+
+; Ensure we don't hoist loads that are modified by callbr.
+ at x = global i32 0
+define i32 @foo3() {
+; CHECK-LABEL: @foo3(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    callbr void asm "", "=*m,!i"(ptr elementtype(i32) @x)
+; CHECK-NEXT:    to label [[ASM_FALLTHROUGH:%.*]] [label %err.split]
+; CHECK:       asm.fallthrough:
+; CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr @x, align 4
+; CHECK-NEXT:    ret i32 [[TMP0]]
+; CHECK:       err.split:
+; CHECK-NEXT:    [[TMP1:%.*]] = load i32, ptr @x, align 4
+; CHECK-NEXT:    ret i32 [[TMP1]]
+;
+entry:
+  callbr void asm "", "=*m,!i"(ptr elementtype(i32) @x)
+  to label %asm.fallthrough [label %err.split]
+
+asm.fallthrough:                                  ; preds = %entry
+  %0 = load i32, ptr @x
+  ret i32 %0
+
+err.split:                                        ; preds = %entry
+  %1 = load i32, ptr @x
+  ret i32 %1
+}
+
+; Ensure we do hoist loads that aren't modified by callbr, if the callbr has
+; the attribute memory(argmem:readwrite).
+ at y = global i32 0
+define i32 @foo4() {
+; CHECK-LABEL: @foo4(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr @y, align 4
+; CHECK-NEXT:    callbr void asm "", "=*m,!i"(ptr elementtype(i32) @x) #[[ATTR2:[0-9]+]]
+; CHECK-NEXT:    to label [[A:%.*]] [label %b]
+; CHECK:       a:
+; CHECK-NEXT:    ret i32 [[TMP0]]
+; CHECK:       b:
+; CHECK-NEXT:    ret i32 [[TMP0]]
+;
+entry:
+  callbr void asm "", "=*m,!i"(ptr elementtype(i32) @x) memory(argmem: readwrite)
+  to label %a [label %b]
+
+a:                                  ; preds = %entry
+  %0 = load i32, ptr @y
+  ret i32 %0
+
+b:                                        ; preds = %entry
+  %1 = load i32, ptr @y
+  ret i32 %1
+}
+
+; Ensure we don't hoist loads that are modified by callbr, if the callbr has
+; the attribute memory(argmem:readwrite).
+define i32 @foo5() {
+; CHECK-LABEL: @foo5(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    callbr void asm "", "=*m,!i"(ptr elementtype(i32) @x) #[[ATTR2]]
+; CHECK-NEXT:    to label [[A:%.*]] [label %b]
+; CHECK:       a:
+; CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr @x, align 4
+; CHECK-NEXT:    ret i32 [[TMP0]]
+; CHECK:       b:
+; CHECK-NEXT:    [[TMP1:%.*]] = load i32, ptr @x, align 4
+; CHECK-NEXT:    ret i32 [[TMP1]]
+;
+entry:
+  callbr void asm "", "=*m,!i"(ptr elementtype(i32) @x) memory(argmem: readwrite)
+  to label %a [label %b]
+
+a:                                  ; preds = %entry
+  %0 = load i32, ptr @x
+  ret i32 %0
+
+b:                                        ; preds = %entry
+  %1 = load i32, ptr @x
+  ret i32 %1
+}
+
+; Ensure we hoist loads that are modified by callbr, if the callbr has the
+; attribute memory(argmem:none).
+define i32 @foo6() {
+; CHECK-LABEL: @foo6(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TMP0:%.*]] = load i32, ptr @x, align 4
+; CHECK-NEXT:    callbr void asm "", "=*m,!i"(ptr elementtype(i32) @x) #[[ATTR3:[0-9]+]]
+; CHECK-NEXT:    to label [[A:%.*]] [label %b]
+; CHECK:       a:
+; CHECK-NEXT:    ret i32 [[TMP0]]
+; CHECK:       b:
+; CHECK-NEXT:    ret i32 [[TMP0]]
+;
+entry:
+  callbr void asm "", "=*m,!i"(ptr elementtype(i32) @x) memory(argmem: none)
+  to label %a [label %b]
+
+a:                                  ; preds = %entry
+  %0 = load i32, ptr @x
+  ret i32 %0
+
+b:                                        ; preds = %entry
+  %1 = load i32, ptr @x
+  ret i32 %1
+}
+


        


More information about the llvm-commits mailing list