[InstCombine] Extend Phi-Icmp use to include or #67682
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-llvm-transforms ChangesIn InstCombinePHI currently the only use of PHI as an Icmp is being checked as a requirement to reduce a value if isKnownNonZero. Full diff: https://github.com/llvm/llvm-project/pull/67682.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
index 00115abf6500597..464c2848b6c3554 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
@@ -1441,36 +1441,56 @@ Instruction *InstCombinerImpl::visitPHINode(PHINode &PN) {
PHIUser->user_back() == &PN) {
return replaceInstUsesWith(PN, PoisonValue::get(PN.getType()));
}
- // When a PHI is used only to be compared with zero, it is safe to replace
- // an incoming value proved as known nonzero with any non-zero constant.
- // For example, in the code below, the incoming value %v can be replaced
- // with any non-zero constant based on the fact that the PHI is only used to
- // be compared with zero and %v is a known non-zero value:
- // %v = select %cond, 1, 2
- // %p = phi [%v, BB] ...
- // icmp eq, %p, 0
- auto *CmpInst = dyn_cast<ICmpInst>(PHIUser);
- // FIXME: To be simple, handle only integer type for now.
- if (CmpInst && isa<IntegerType>(PN.getType()) && CmpInst->isEquality() &&
- match(CmpInst->getOperand(1), m_Zero())) {
- ConstantInt *NonZeroConst = nullptr;
- bool MadeChange = false;
- for (unsigned I = 0, E = PN.getNumIncomingValues(); I != E; ++I) {
- Instruction *CtxI = PN.getIncomingBlock(I)->getTerminator();
- Value *VA = PN.getIncomingValue(I);
- if (isKnownNonZero(VA, DL, 0, &AC, CtxI, &DT)) {
- if (!NonZeroConst)
- NonZeroConst = getAnyNonZeroConstInt(PN);
+ }
- if (NonZeroConst != VA) {
- replaceOperand(PN, I, NonZeroConst);
- MadeChange = true;
- }
+ // When a PHI is used only to be compared with zero, it is safe to replace
+ // an incoming value proved as known nonzero with any non-zero constant.
+ // For example, in the code below, the incoming value %v can be replaced
+ // with any non-zero constant based on the fact that the PHI is only used to
+ // be compared with zero and %v is a known non-zero value:
+ // %v = select %cond, 1, 2
+ // %p = phi [%v, BB] ...
+ // icmp eq, %p, 0
+ // FIXME: To be simple, handle only integer type for now.
+ // Extend to 2 use of phi -> icmp and or(icmp)
+ bool AllUsesOfPhiEndsInCmp = false;
+ if (PN.hasOneUse() || PN.hasNUses(2)) {
+ for (const auto *U : PN.users()) {
+ auto *CmpInst = dyn_cast<ICmpInst>(U);
+ if (!CmpInst) {
+ // This is always correct as OR only add bits and we are checking
+ // against 0.
+ if (U->hasOneUse() && match(U, m_Or(m_Specific(&PN), m_Value())))
+ CmpInst = dyn_cast<ICmpInst>(U->user_back());
+ }
+ if (CmpInst && isa<IntegerType>(PN.getType()) && CmpInst->isEquality() &&
+ match(CmpInst->getOperand(1), m_Zero()))
+ AllUsesOfPhiEndsInCmp = true;
+ else {
+ AllUsesOfPhiEndsInCmp = false;
+ break;
+ }
+ }
+ }
+
+ // All uses of PHI results in a compare with zero.
+ if (AllUsesOfPhiEndsInCmp) {
+ ConstantInt *NonZeroConst = nullptr;
+ bool MadeChange = false;
+ for (unsigned I = 0, E = PN.getNumIncomingValues(); I != E; ++I) {
+ Instruction *CtxI = PN.getIncomingBlock(I)->getTerminator();
+ Value *VA = PN.getIncomingValue(I);
+ if (isKnownNonZero(VA, DL, 0, &AC, CtxI, &DT)) {
+ if (!NonZeroConst)
+ NonZeroConst = getAnyNonZeroConstInt(PN);
+ if (NonZeroConst != VA) {
+ replaceOperand(PN, I, NonZeroConst);
+ MadeChange = true;
}
}
- if (MadeChange)
- return &PN;
}
+ if (MadeChange)
+ return &PN;
}
// We sometimes end up with phi cycles that non-obviously end up being the
diff --git a/llvm/test/Transforms/InstCombine/phi.ll b/llvm/test/Transforms/InstCombine/phi.ll
index a5cab8a71d3f711..3c0d4c9b055bcce 100644
--- a/llvm/test/Transforms/InstCombine/phi.ll
+++ b/llvm/test/Transforms/InstCombine/phi.ll
@@ -1286,6 +1286,127 @@ if.end: ; preds = %entry, %if.then
ret i1 %cmp1
}
+define i1 @phi_knownnonzero_eq_oricmp(i32 %n, i32 %s, ptr nocapture readonly %P, i32 %val) {
+; CHECK-LABEL: @phi_knownnonzero_eq_oricmp(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[TOBOOL:%.*]] = icmp slt i32 [[N:%.*]], [[S:%.*]]
+; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
+; CHECK: if.then:
+; CHECK-NEXT: br label [[IF_END]]
+; CHECK: if.end:
+; CHECK-NEXT: [[A_0:%.*]] = phi i32 [ 1, [[IF_THEN]] ], [ [[N]], [[ENTRY:%.*]] ]
+; CHECK-NEXT: [[TMP0:%.*]] = or i32 [[A_0]], [[VAL:%.*]]
+; CHECK-NEXT: [[CMP1:%.*]] = icmp eq i32 [[TMP0]], 0
+; CHECK-NEXT: ret i1 [[CMP1]]
+;
+entry:
+ %tobool = icmp slt i32 %n, %s
+ br i1 %tobool, label %if.end, label %if.then
+
+if.then: ; preds = %entry
+ %0 = load i32, ptr %P
+ %cmp = icmp eq i32 %n, %0
+ %1 = select i1 %cmp, i32 1, i32 2
+ br label %if.end
+
+if.end: ; preds = %entry, %if.then
+ %a.0 = phi i32 [ %1, %if.then ], [ %n, %entry ]
+ %2 = or i32 %a.0, %val
+ %cmp1 = icmp eq i32 %2, 0
+ ret i1 %cmp1
+}
+
+define i1 @phi_knownnonzero_eq_multiuse_oricmp(i32 %n, i32 %s, ptr nocapture readonly %P, i32 %val) {
+; CHECK-LABEL: @phi_knownnonzero_eq_multiuse_oricmp(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[TOBOOL:%.*]] = icmp slt i32 [[N:%.*]], [[S:%.*]]
+; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
+; CHECK: if.then:
+; CHECK-NEXT: br label [[IF_END]]
+; CHECK: if.end:
+; CHECK-NEXT: [[A_0:%.*]] = phi i32 [ 1, [[IF_THEN]] ], [ [[N]], [[ENTRY:%.*]] ]
+; CHECK-NEXT: [[TMP0:%.*]] = or i32 [[A_0]], [[VAL:%.*]]
+; CHECK-NEXT: [[CMP1:%.*]] = icmp eq i32 [[TMP0]], 0
+; CHECK-NEXT: br i1 [[CMP1]], label [[NEXT:%.*]], label [[CLEANUP:%.*]]
+; CHECK: next:
+; CHECK-NEXT: [[BOOL2:%.*]] = icmp eq i32 [[A_0]], 0
+; CHECK-NEXT: br label [[CLEANUP]]
+; CHECK: cleanup:
+; CHECK-NEXT: [[FINAL:%.*]] = phi i1 [ [[CMP1]], [[IF_END]] ], [ [[BOOL2]], [[NEXT]] ]
+; CHECK-NEXT: ret i1 [[FINAL]]
+;
+entry:
+ %tobool = icmp slt i32 %n, %s
+ br i1 %tobool, label %if.end, label %if.then
+
+if.then: ; preds = %entry
+ %0 = load i32, ptr %P
+ %cmp = icmp eq i32 %n, %0
+ %1 = select i1 %cmp, i32 1, i32 2
+ br label %if.end
+
+if.end: ; preds = %entry, %if.then
+ %a.0 = phi i32 [ %1, %if.then ], [ %n, %entry ]
+ %2 = or i32 %a.0, %val
+ %cmp1 = icmp eq i32 %2, 0
+ br i1 %cmp1, label %next, label %cleanup
+
+next:
+ %bool2 = icmp eq i32 %a.0, 0
+ br label %cleanup
+
+cleanup:
+ %final = phi i1 [ %cmp1, %if.end ], [ %bool2, %next ]
+ ret i1 %final
+}
+
+define i1 @phi_knownnonzero_eq_multiuse_andicmp(i32 %n, i32 %s, ptr nocapture readonly %P, i32 %val) {
+; CHECK-LABEL: @phi_knownnonzero_eq_multiuse_andicmp(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[TOBOOL:%.*]] = icmp slt i32 [[N:%.*]], [[S:%.*]]
+; CHECK-NEXT: br i1 [[TOBOOL]], label [[IF_END:%.*]], label [[IF_THEN:%.*]]
+; CHECK: if.then:
+; CHECK-NEXT: [[TMP0:%.*]] = load i32, ptr [[P:%.*]], align 4
+; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[TMP0]], [[N]]
+; CHECK-NEXT: [[TMP1:%.*]] = select i1 [[CMP]], i32 1, i32 2
+; CHECK-NEXT: br label [[IF_END]]
+; CHECK: if.end:
+; CHECK-NEXT: [[A_0:%.*]] = phi i32 [ [[TMP1]], [[IF_THEN]] ], [ [[N]], [[ENTRY:%.*]] ]
+; CHECK-NEXT: [[TMP2:%.*]] = and i32 [[A_0]], [[VAL:%.*]]
+; CHECK-NEXT: [[CMP1:%.*]] = icmp eq i32 [[TMP2]], 0
+; CHECK-NEXT: br i1 [[CMP1]], label [[NEXT:%.*]], label [[CLEANUP:%.*]]
+; CHECK: next:
+; CHECK-NEXT: [[BOOL2:%.*]] = icmp eq i32 [[A_0]], 0
+; CHECK-NEXT: br label [[CLEANUP]]
+; CHECK: cleanup:
+; CHECK-NEXT: [[FINAL:%.*]] = phi i1 [ [[CMP1]], [[IF_END]] ], [ [[BOOL2]], [[NEXT]] ]
+; CHECK-NEXT: ret i1 [[FINAL]]
+;
+entry:
+ %tobool = icmp slt i32 %n, %s
+ br i1 %tobool, label %if.end, label %if.then
+
+if.then: ; preds = %entry
+ %0 = load i32, ptr %P
+ %cmp = icmp eq i32 %n, %0
+ %1 = select i1 %cmp, i32 1, i32 2
+ br label %if.end
+
+if.end: ; preds = %entry, %if.then
+ %a.0 = phi i32 [ %1, %if.then ], [ %n, %entry ]
+ %2 = and i32 %a.0, %val
+ %cmp1 = icmp eq i32 %2, 0
+ br i1 %cmp1, label %next, label %cleanup
+
+next:
+ %bool2 = icmp eq i32 %a.0, 0
+ br label %cleanup
+
+cleanup:
+ %final = phi i1 [ %cmp1, %if.end ], [ %bool2, %next ]
+ ret i1 %final
+}
+
; This would crash trying to delete an instruction (conv)
; that still had uses because the user (the phi) was not
; updated to remove a use from an unreachable block (g.exit).
|
davemgreen
reviewed
Oct 1, 2023
davemgreen
reviewed
Oct 5, 2023
|
@nikic Thanks for your previous review. Let me know if there if anything else required for the patch. Thanks. |
|
Gentle Ping |
|
Ping |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In InstCombinePHI currently the only use of PHI as an Icmp is being checked as a requirement to reduce a value if isKnownNonZero.
However this can be extended to include or(icmp) . This is always true as OR only adds bits and we are checking against 0.