[llvm] 514bc01 - [SimplifyCFG] FoldBranchToCommonDest(): properly handle same-block external uses (PR49510/PR49689)

[Roman Lebedev via llvm-commits]

Author: Roman Lebedev
Date: 2021-03-23T17:37:28+03:00
New Revision: 514bc01ca3b9f97945dd49aa40e01922fed0e41d

URL: https://github.com/llvm/llvm-project/commit/514bc01ca3b9f97945dd49aa40e01922fed0e41d
DIFF: https://github.com/llvm/llvm-project/commit/514bc01ca3b9f97945dd49aa40e01922fed0e41d.diff

LOG: [SimplifyCFG] FoldBranchToCommonDest(): properly handle same-block external uses (PR49510/PR49689)

We clone bonus instructions to the end of the predecessor block,
and then use `SSAUpdater::RewriteUseAfterInsertions()`.
But that only deals with the cases where the use-to-be-rewritten
are either in different block from the def, or come after the def.

But in some loop cases, the external use may be in the beginning of
predecessor block, before the newly cloned bonus instruction.
`SSAUpdater::RewriteUseAfterInsertions()` does not deal with that.
Notably, the external use can't happen to be both in the same block
and *after* the newly-cloned instruction, because of the fold preconditions.

To properly handle these cases, when the use is in the same block,
we should instead use `SSAUpdater::RewriteUse()`.
TBN, they do the same thing for PHI users.

Fixes https://bugs.llvm.org/show_bug.cgi?id=49510
Likely Fixes https://bugs.llvm.org/show_bug.cgi?id=49689

Added: 
    

Modified: 
    llvm/lib/Transforms/Utils/SimplifyCFG.cpp
    llvm/test/Transforms/SimplifyCFG/fold-branch-to-common-dest.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 6190367c6498..0b1215e597cd 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1097,8 +1097,13 @@ static void CloneInstructionsIntoPredecessorBlockAndUpdateSSAUses(
                          (NewBonusInst->getName() + ".merge").str());
     SSAUpdate.AddAvailableValue(BB, &BonusInst);
     SSAUpdate.AddAvailableValue(PredBlock, NewBonusInst);
-    for (Use &U : make_early_inc_range(BonusInst.uses()))
-      SSAUpdate.RewriteUseAfterInsertions(U);
+    for (Use &U : make_early_inc_range(BonusInst.uses())) {
+      auto *UI = cast<Instruction>(U.getUser());
+      if (UI->getParent() != PredBlock)
+        SSAUpdate.RewriteUseAfterInsertions(U);
+      else // Use is in the same block as, and comes before, NewBonusInst.
+        SSAUpdate.RewriteUse(U);
+    }
   }
 }
 

diff  --git a/llvm/test/Transforms/SimplifyCFG/fold-branch-to-common-dest.ll b/llvm/test/Transforms/SimplifyCFG/fold-branch-to-common-dest.ll
index 1f4cdb26b356..b578bb1bd45e 100644
--- a/llvm/test/Transforms/SimplifyCFG/fold-branch-to-common-dest.ll
+++ b/llvm/test/Transforms/SimplifyCFG/fold-branch-to-common-dest.ll
@@ -894,3 +894,37 @@ if.end7:
 cleanup:
   unreachable
 }
+
+ at global_pr49510 = external global i16, align 1
+
+define void @pr49510() {
+; CHECK-LABEL: @pr49510(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[DOTOLD:%.*]] = load i16, i16* @global_pr49510, align 1
+; CHECK-NEXT:    [[TOBOOL_OLD:%.*]] = icmp ne i16 [[DOTOLD]], 0
+; CHECK-NEXT:    br i1 [[TOBOOL_OLD]], label [[LAND_RHS:%.*]], label [[FOR_END:%.*]]
+; CHECK:       land.rhs:
+; CHECK-NEXT:    [[DOTMERGE:%.*]] = phi i16 [ [[TMP0:%.*]], [[LAND_RHS]] ], [ [[DOTOLD]], [[ENTRY:%.*]] ]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp slt i16 [[DOTMERGE]], 0
+; CHECK-NEXT:    [[TMP0]] = load i16, i16* @global_pr49510, align 1
+; CHECK-NEXT:    [[TOBOOL:%.*]] = icmp ne i16 [[TMP0]], 0
+; CHECK-NEXT:    [[OR_COND:%.*]] = select i1 [[CMP]], i1 [[TOBOOL]], i1 false
+; CHECK-NEXT:    br i1 [[OR_COND]], label [[LAND_RHS]], label [[FOR_END]]
+; CHECK:       for.end:
+; CHECK-NEXT:    ret void
+;
+entry:
+  br label %for.cond
+
+for.cond:
+  %0 = load i16, i16* @global_pr49510, align 1
+  %tobool = icmp ne i16 %0, 0
+  br i1 %tobool, label %land.rhs, label %for.end
+
+land.rhs:
+  %cmp = icmp slt i16 %0, 0
+  br i1 %cmp, label %for.cond, label %for.end
+
+for.end:
+  ret void
+}