Skip to content
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

[ConstraintElim] Support adding facts from switch terminators. #67061

Merged
merged 1 commit into from
Sep 22, 2023

Conversation

fhahn
Copy link
Contributor

@fhahn fhahn commented Sep 21, 2023

After 4a5bcbd, switch instructions can now be handled in a straight-forward manner by adding (ICMP_EQ, ConditionVal, CaseVal) for te successor blocks per case.

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 21, 2023

@llvm/pr-subscribers-llvm-transforms

Changes

After 4a5bcbd, switch instructions can now be handled in a straight-forward manner by adding (ICMP_EQ, ConditionVal, CaseVal) for te successor blocks per case.


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

2 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/ConstraintElimination.cpp (+12)
  • (modified) llvm/test/Transforms/ConstraintElimination/switch.ll (+3-9)
diff --git a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
index 3c47d36cbc0a0bc..fd6b6b553348511 100644
--- a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
@@ -831,6 +831,18 @@ void State::addInfoFor(BasicBlock &BB) {
     GuaranteedToExecute &= isGuaranteedToTransferExecutionToSuccessor(&I);
   }
 
+  if (auto *Switch = dyn_cast<SwitchInst>(BB.getTerminator())) {
+    for (auto &Case : Switch->cases()) {
+      BasicBlock *Succ = Case.getCaseSuccessor();
+      Value *V = Case.getCaseValue();
+      if (!canAddSuccessor(BB, Succ))
+        continue;
+      WorkList.emplace_back(FactOrCheck::getConditionFact(
+          DT.getNode(Succ), CmpInst::ICMP_EQ, Switch->getCondition(), V));
+    }
+    return;
+  }
+
   auto *Br = dyn_cast<BranchInst>(BB.getTerminator());
   if (!Br || !Br->isConditional())
     return;
diff --git a/llvm/test/Transforms/ConstraintElimination/switch.ll b/llvm/test/Transforms/ConstraintElimination/switch.ll
index 25e6d34f0250c45..8eae227f7334641 100644
--- a/llvm/test/Transforms/ConstraintElimination/switch.ll
+++ b/llvm/test/Transforms/ConstraintElimination/switch.ll
@@ -57,14 +57,10 @@ define i1 @simplify_based_on_switch(i32 %x) {
 ; CHECK-NEXT:    [[RES_1:%.*]] = xor i1 [[C_1]], [[C_2]]
 ; CHECK-NEXT:    ret i1 [[RES_1]]
 ; CHECK:       exit.2:
-; CHECK-NEXT:    [[T_1:%.*]] = icmp ult i32 [[X]], 7
-; CHECK-NEXT:    [[F_1:%.*]] = icmp ult i32 [[X]], 6
-; CHECK-NEXT:    [[RES_2:%.*]] = xor i1 [[T_1]], [[F_1]]
+; CHECK-NEXT:    [[RES_2:%.*]] = xor i1 true, false
 ; CHECK-NEXT:    ret i1 [[RES_2]]
 ; CHECK:       exit.3:
-; CHECK-NEXT:    [[T_2:%.*]] = icmp ult i32 [[X]], 11
-; CHECK-NEXT:    [[F_2:%.*]] = icmp ult i32 [[X]], 10
-; CHECK-NEXT:    [[RES_3:%.*]] = xor i1 [[T_2]], [[F_2]]
+; CHECK-NEXT:    [[RES_3:%.*]] = xor i1 true, false
 ; CHECK-NEXT:    ret i1 [[RES_3]]
 ;
 entry:
@@ -105,9 +101,7 @@ define i1 @simplify_based_on_switch_successor_branches(i32 %x) {
 ; CHECK-NEXT:    [[RES_1:%.*]] = xor i1 [[C_1]], [[C_2]]
 ; CHECK-NEXT:    ret i1 [[RES_1]]
 ; CHECK:       exit.2:
-; CHECK-NEXT:    [[T_1:%.*]] = icmp ult i32 [[X]], 7
-; CHECK-NEXT:    [[F_1:%.*]] = icmp ult i32 [[X]], 6
-; CHECK-NEXT:    [[RES_2:%.*]] = xor i1 [[T_1]], [[F_1]]
+; CHECK-NEXT:    [[RES_2:%.*]] = xor i1 true, false
 ; CHECK-NEXT:    call void @use(i1 [[RES_2]])
 ; CHECK-NEXT:    br label [[EXIT_3]]
 ; CHECK:       exit.3:

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but can you please also add a negative test with a multi-edge from the switch? I.e. two cases going to the same block. I think the dominates query handles this correctly, but better be sure.

fhahn added a commit that referenced this pull request Sep 22, 2023
Shorten the types used to i8 for cheaper verification and add test case
where 2 cases have the same destination, as suggested in #67061.
After 4a5bcbd, switch instructions can now be handled in a
straight-forward manner by adding (ICMP_EQ, ConditionVal, CaseVal) for
te successor blocks per case.
@fhahn fhahn merged commit 39d7f70 into llvm:main Sep 22, 2023
2 of 3 checks passed
@fhahn fhahn deleted the pr-ce-switch branch September 22, 2023 10:18
@fhahn
Copy link
Contributor Author

fhahn commented Sep 22, 2023

LGTM, but can you please also add a negative test with a multi-edge from the switch? I.e. two cases going to the same block. I think the dominates query handles this correctly, but better be sure.

Thanks, added in 0cb3530

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.

None yet

3 participants