[llvm] r281162 - [SimplifyCFG] Be even more conservative in SinkThenElseCodeToEnd

James Molloy via llvm-commits llvm-commits at lists.llvm.org
Sun Sep 11 02:00:03 PDT 2016


Author: jamesm
Date: Sun Sep 11 04:00:03 2016
New Revision: 281162

URL: http://llvm.org/viewvc/llvm-project?rev=281162&view=rev
Log:
[SimplifyCFG] Be even more conservative in SinkThenElseCodeToEnd

This should *actually* fix PR30244. This cranks up the workaround for PR30188 so that we never sink loads or stores of allocas.

The idea is that these should be removed by SROA/Mem2Reg, and any movement of them may well confuse SROA or just cause unwanted code churn. It's not ideal that the midend should be crippled like this, but that unwanted churn can really cause significant regressions in important workloads (tsan).

Modified:
    llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
    llvm/trunk/test/Transforms/SimplifyCFG/inline-asm-sink.ll
    llvm/trunk/test/Transforms/SimplifyCFG/sink-common-code.ll

Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=281162&r1=281161&r2=281162&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Sun Sep 11 04:00:03 2016
@@ -1428,6 +1428,25 @@ static bool canSinkInstructions(
     if (I0->getOperand(OI)->getType()->isTokenTy())
       // Don't touch any operand of token type.
       return false;
+
+    // Because SROA can't handle speculating stores of selects, try not
+    // to sink loads or stores of allocas when we'd have to create a PHI for
+    // the address operand. Also, because it is likely that loads or stores
+    // of allocas will disappear when Mem2Reg/SROA is run, don't sink them.
+    // This can cause code churn which can have unintended consequences down
+    // the line - see https://llvm.org/bugs/show_bug.cgi?id=30244.
+    // FIXME: This is a workaround for a deficiency in SROA - see
+    // https://llvm.org/bugs/show_bug.cgi?id=30188
+    if (OI == 1 && isa<StoreInst>(I0) &&
+        any_of(Insts, [](const Instruction *I) {
+          return isa<AllocaInst>(I->getOperand(1));
+        }))
+      return false;
+    if (OI == 0 && isa<LoadInst>(I0) && any_of(Insts, [](const Instruction *I) {
+          return isa<AllocaInst>(I->getOperand(0));
+        }))
+      return false;
+
     auto SameAsI0 = [&I0, OI](const Instruction *I) {
       assert(I->getNumOperands() == I0->getNumOperands());
       return I->getOperand(OI) == I0->getOperand(OI);
@@ -1441,21 +1460,6 @@ static bool canSinkInstructions(
         // FIXME: if the call was *already* indirect, we should do this.
         return false;
       }
-      // Because SROA can't handle speculating stores of selects, try not
-      // to sink loads or stores of allocas when we'd have to create a PHI for
-      // the address operand.
-      // FIXME: This is a workaround for a deficiency in SROA - see
-      // https://llvm.org/bugs/show_bug.cgi?id=30188
-      if (OI == 1 && isa<StoreInst>(I0) &&
-          any_of(Insts, [](const Instruction *I) {
-            return isa<AllocaInst>(I->getOperand(1));
-          }))
-        return false;
-      if (OI == 0 && isa<LoadInst>(I0) &&
-          any_of(Insts, [](const Instruction *I) {
-            return isa<AllocaInst>(I->getOperand(0));
-          }))
-        return false;
       for (auto *I : Insts)
         PHIOperands[I].push_back(I->getOperand(OI));
     }

Modified: llvm/trunk/test/Transforms/SimplifyCFG/inline-asm-sink.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/inline-asm-sink.ll?rev=281162&r1=281161&r2=281162&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/SimplifyCFG/inline-asm-sink.ll (original)
+++ llvm/trunk/test/Transforms/SimplifyCFG/inline-asm-sink.ll Sun Sep 11 04:00:03 2016
@@ -1,4 +1,4 @@
-; RUN: opt < %s -simplifycfg -S | FileCheck %s
+; RUN: opt < %s -mem2reg -simplifycfg -S | FileCheck %s
 
 define i32 @test(i32 %x) {
 ; CHECK-LABEL: @test
@@ -23,7 +23,7 @@ if.else:
 
 if.end:
 ; CHECK-LABEL: if.end:
-; CHECK: {{%.*}} = phi i32 [ [[ASM2]], %if.else ], [ [[ASM1]], %if.then ]
+; CHECK: {{%.*}} = phi i32 [ [[ASM1]], %if.then ], [ [[ASM2]], %if.else ]
   %tmp3 = load i32, i32* %y, align 4
   ret i32 %tmp3
 }

Modified: llvm/trunk/test/Transforms/SimplifyCFG/sink-common-code.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/sink-common-code.ll?rev=281162&r1=281161&r2=281162&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/SimplifyCFG/sink-common-code.ll (original)
+++ llvm/trunk/test/Transforms/SimplifyCFG/sink-common-code.ll Sun Sep 11 04:00:03 2016
@@ -669,24 +669,35 @@ declare void @g()
 ; CHECK-LABEL: test_pr30292
 ; CHECK: phi i32 [ 0, %entry ], [ %add1, %succ ], [ %add2, %two ]
 
-define void @test_pr30244(i1 %cond, i1 %cond2, i32 %a, i32 %b) {
+define zeroext i1 @test_pr30244(i1 zeroext %flag, i1 zeroext %flag2, i32 %blksA, i32 %blksB, i32 %nblks) {
+
 entry:
-  %add1 = add i32 %a, 1
-  br label %succ
+  %p = alloca i8
+  br i1 %flag, label %if.then, label %if.else
 
-one:
-  br i1 %cond, label %two, label %succ
+if.then:
+  %cmp = icmp uge i32 %blksA, %nblks
+  %frombool1 = zext i1 %cmp to i8
+  store i8 %frombool1, i8* %p
+  br label %if.end
+
+if.else:
+  br i1 %flag2, label %if.then2, label %if.end
+
+if.then2:
+  %add = add i32 %nblks, %blksB
+  %cmp2 = icmp ule i32 %add, %blksA
+  %frombool3 = zext i1 %cmp2 to i8
+  store i8 %frombool3, i8* %p
+  br label %if.end
 
-two:
-  call void @g()
-  %add2 = add i32 %a, 1
-  br label %succ
-
-succ:
-  %p = phi i32 [ 0, %entry ], [ %add1, %one ], [ %add2, %two ]
-  br label %one
+if.end:
+  ret i1 true
 }
 
+; CHECK-LABEL: @test_pr30244
+; CHECK: store
+; CHECK: store
 
 ; CHECK: !0 = !{!1, !1, i64 0}
 ; CHECK: !1 = !{!"float", !2}




More information about the llvm-commits mailing list