[InstCombine] Support reassoc for foldLogicOfFCmps #116065
Merged
Conversation
This file contains hidden or 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 Author: Nikita Popov (nikic) ChangesWe currently support simple reassociation for foldAndOrOfICmps(). Support the same for foldLogicOfFCmps() by going through the common foldBooleanAndOr() helper. This will also resolve the regression on #112704, which is also due to missing reassoc support. I had to adjust one fold to add support for FMF flag preservation, otherwise there would be test regressions. There is a separate fold (reassociateFCmps) handling reassociation for just that specific case and it preserves FMF. Unfortuantely it's not rendered entirely redundant by this patch, because it handles one more level of reassociation as well. Full diff: https://github.com/llvm/llvm-project/pull/116065.diff 3 Files Affected:
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index 50d1c61c24cf47..882604e79a2dc4 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -1465,11 +1465,16 @@ Value *InstCombinerImpl::foldLogicOfFCmps(FCmpInst *LHS, FCmpInst *RHS,
// FCmp canonicalization ensures that (fcmp ord/uno X, X) and
// (fcmp ord/uno X, C) will be transformed to (fcmp X, +0.0).
- if (match(LHS1, m_PosZeroFP()) && match(RHS1, m_PosZeroFP()))
+ if (match(LHS1, m_PosZeroFP()) && match(RHS1, m_PosZeroFP())) {
// Ignore the constants because they are obviously not NANs:
// (fcmp ord x, 0.0) & (fcmp ord y, 0.0) -> (fcmp ord x, y)
// (fcmp uno x, 0.0) | (fcmp uno y, 0.0) -> (fcmp uno x, y)
+ IRBuilder<>::FastMathFlagGuard FMFG(Builder);
+ FastMathFlags FMF = LHS->getFastMathFlags();
+ FMF &= RHS->getFastMathFlags();
+ Builder.setFastMathFlags(FMF);
return Builder.CreateFCmp(PredL, LHS0, RHS0);
+ }
}
if (IsAnd && stripSignOnlyFPOps(LHS0) == stripSignOnlyFPOps(RHS0)) {
@@ -2728,47 +2733,35 @@ Instruction *InstCombinerImpl::visitAnd(BinaryOperator &I) {
foldBooleanAndOr(Op0, Op1, I, /*IsAnd=*/true, /*IsLogical=*/false))
return replaceInstUsesWith(I, Res);
- {
- ICmpInst *LHS = dyn_cast<ICmpInst>(Op0);
- ICmpInst *RHS = dyn_cast<ICmpInst>(Op1);
-
- // TODO: Base this on foldBooleanAndOr instead?
- // TODO: Make this recursive; it's a little tricky because an arbitrary
- // number of 'and' instructions might have to be created.
- if (LHS && match(Op1, m_OneUse(m_LogicalAnd(m_Value(X), m_Value(Y))))) {
- bool IsLogical = isa<SelectInst>(Op1);
- // LHS & (X && Y) --> (LHS && X) && Y
- if (auto *Cmp = dyn_cast<ICmpInst>(X))
- if (Value *Res =
- foldAndOrOfICmps(LHS, Cmp, I, /* IsAnd */ true, IsLogical))
- return replaceInstUsesWith(I, IsLogical
- ? Builder.CreateLogicalAnd(Res, Y)
- : Builder.CreateAnd(Res, Y));
- // LHS & (X && Y) --> X && (LHS & Y)
- if (auto *Cmp = dyn_cast<ICmpInst>(Y))
- if (Value *Res = foldAndOrOfICmps(LHS, Cmp, I, /* IsAnd */ true,
- /* IsLogical */ false))
- return replaceInstUsesWith(I, IsLogical
- ? Builder.CreateLogicalAnd(X, Res)
- : Builder.CreateAnd(X, Res));
- }
- if (RHS && match(Op0, m_OneUse(m_LogicalAnd(m_Value(X), m_Value(Y))))) {
- bool IsLogical = isa<SelectInst>(Op0);
- // (X && Y) & RHS --> (X && RHS) && Y
- if (auto *Cmp = dyn_cast<ICmpInst>(X))
- if (Value *Res =
- foldAndOrOfICmps(Cmp, RHS, I, /* IsAnd */ true, IsLogical))
- return replaceInstUsesWith(I, IsLogical
- ? Builder.CreateLogicalAnd(Res, Y)
- : Builder.CreateAnd(Res, Y));
- // (X && Y) & RHS --> X && (Y & RHS)
- if (auto *Cmp = dyn_cast<ICmpInst>(Y))
- if (Value *Res = foldAndOrOfICmps(Cmp, RHS, I, /* IsAnd */ true,
- /* IsLogical */ false))
- return replaceInstUsesWith(I, IsLogical
- ? Builder.CreateLogicalAnd(X, Res)
- : Builder.CreateAnd(X, Res));
- }
+ // TODO: Make this recursive; it's a little tricky because an arbitrary
+ // number of 'and' instructions might have to be created.
+ if (match(Op1, m_OneUse(m_LogicalAnd(m_Value(X), m_Value(Y))))) {
+ bool IsLogical = isa<SelectInst>(Op1);
+ // Op0 & (X && Y) --> (Op0 && X) && Y
+ if (Value *Res = foldBooleanAndOr(Op0, X, I, /* IsAnd */ true, IsLogical))
+ return replaceInstUsesWith(I, IsLogical
+ ? Builder.CreateLogicalAnd(Res, Y)
+ : Builder.CreateAnd(Res, Y));
+ // Op0 & (X && Y) --> X && (Op0 & Y)
+ if (Value *Res = foldBooleanAndOr(Op0, Y, I, /* IsAnd */ true,
+ /* IsLogical */ false))
+ return replaceInstUsesWith(I, IsLogical
+ ? Builder.CreateLogicalAnd(X, Res)
+ : Builder.CreateAnd(X, Res));
+ }
+ if (match(Op0, m_OneUse(m_LogicalAnd(m_Value(X), m_Value(Y))))) {
+ bool IsLogical = isa<SelectInst>(Op0);
+ // (X && Y) & Op1 --> (X && Op1) && Y
+ if (Value *Res = foldBooleanAndOr(X, Op1, I, /* IsAnd */ true, IsLogical))
+ return replaceInstUsesWith(I, IsLogical
+ ? Builder.CreateLogicalAnd(Res, Y)
+ : Builder.CreateAnd(Res, Y));
+ // (X && Y) & Op1 --> X && (Y & Op1)
+ if (Value *Res = foldBooleanAndOr(Y, Op1, I, /* IsAnd */ true,
+ /* IsLogical */ false))
+ return replaceInstUsesWith(I, IsLogical
+ ? Builder.CreateLogicalAnd(X, Res)
+ : Builder.CreateAnd(X, Res));
}
if (Instruction *FoldedFCmps = reassociateFCmps(I, Builder))
@@ -3827,48 +3820,31 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {
foldBooleanAndOr(Op0, Op1, I, /*IsAnd=*/false, /*IsLogical=*/false))
return replaceInstUsesWith(I, Res);
- {
- ICmpInst *LHS = dyn_cast<ICmpInst>(Op0);
- ICmpInst *RHS = dyn_cast<ICmpInst>(Op1);
-
- // TODO: Base this on foldBooleanAndOr instead?
- // TODO: Make this recursive; it's a little tricky because an arbitrary
- // number of 'or' instructions might have to be created.
- Value *X, *Y;
- if (LHS && match(Op1, m_OneUse(m_LogicalOr(m_Value(X), m_Value(Y))))) {
- bool IsLogical = isa<SelectInst>(Op1);
- // LHS | (X || Y) --> (LHS || X) || Y
- if (auto *Cmp = dyn_cast<ICmpInst>(X))
- if (Value *Res =
- foldAndOrOfICmps(LHS, Cmp, I, /* IsAnd */ false, IsLogical))
- return replaceInstUsesWith(I, IsLogical
- ? Builder.CreateLogicalOr(Res, Y)
- : Builder.CreateOr(Res, Y));
- // LHS | (X || Y) --> X || (LHS | Y)
- if (auto *Cmp = dyn_cast<ICmpInst>(Y))
- if (Value *Res = foldAndOrOfICmps(LHS, Cmp, I, /* IsAnd */ false,
- /* IsLogical */ false))
- return replaceInstUsesWith(I, IsLogical
- ? Builder.CreateLogicalOr(X, Res)
- : Builder.CreateOr(X, Res));
- }
- if (RHS && match(Op0, m_OneUse(m_LogicalOr(m_Value(X), m_Value(Y))))) {
- bool IsLogical = isa<SelectInst>(Op0);
- // (X || Y) | RHS --> (X || RHS) || Y
- if (auto *Cmp = dyn_cast<ICmpInst>(X))
- if (Value *Res =
- foldAndOrOfICmps(Cmp, RHS, I, /* IsAnd */ false, IsLogical))
- return replaceInstUsesWith(I, IsLogical
- ? Builder.CreateLogicalOr(Res, Y)
- : Builder.CreateOr(Res, Y));
- // (X || Y) | RHS --> X || (Y | RHS)
- if (auto *Cmp = dyn_cast<ICmpInst>(Y))
- if (Value *Res = foldAndOrOfICmps(Cmp, RHS, I, /* IsAnd */ false,
- /* IsLogical */ false))
- return replaceInstUsesWith(I, IsLogical
- ? Builder.CreateLogicalOr(X, Res)
- : Builder.CreateOr(X, Res));
- }
+ // TODO: Make this recursive; it's a little tricky because an arbitrary
+ // number of 'or' instructions might have to be created.
+ if (match(Op1, m_OneUse(m_LogicalOr(m_Value(X), m_Value(Y))))) {
+ bool IsLogical = isa<SelectInst>(Op1);
+ // Op0 | (X || Y) --> (Op0 || X) || Y
+ if (Value *Res = foldBooleanAndOr(Op0, X, I, /* IsAnd */ false, IsLogical))
+ return replaceInstUsesWith(I, IsLogical ? Builder.CreateLogicalOr(Res, Y)
+ : Builder.CreateOr(Res, Y));
+ // Op0 | (X || Y) --> X || (Op0 | Y)
+ if (Value *Res = foldBooleanAndOr(Op0, Y, I, /* IsAnd */ false,
+ /* IsLogical */ false))
+ return replaceInstUsesWith(I, IsLogical ? Builder.CreateLogicalOr(X, Res)
+ : Builder.CreateOr(X, Res));
+ }
+ if (match(Op0, m_OneUse(m_LogicalOr(m_Value(X), m_Value(Y))))) {
+ bool IsLogical = isa<SelectInst>(Op0);
+ // (X || Y) | Op1 --> (X || Op1) || Y
+ if (Value *Res = foldBooleanAndOr(X, Op1, I, /* IsAnd */ false, IsLogical))
+ return replaceInstUsesWith(I, IsLogical ? Builder.CreateLogicalOr(Res, Y)
+ : Builder.CreateOr(Res, Y));
+ // (X || Y) | Op1 --> X || (Y | Op1)
+ if (Value *Res = foldBooleanAndOr(Y, Op1, I, /* IsAnd */ false,
+ /* IsLogical */ false))
+ return replaceInstUsesWith(I, IsLogical ? Builder.CreateLogicalOr(X, Res)
+ : Builder.CreateOr(X, Res));
}
if (Instruction *FoldedFCmps = reassociateFCmps(I, Builder))
diff --git a/llvm/test/Transforms/InstCombine/and-fcmp.ll b/llvm/test/Transforms/InstCombine/and-fcmp.ll
index 30b9fca6e97ada..c7bbc8ab56f9a6 100644
--- a/llvm/test/Transforms/InstCombine/and-fcmp.ll
+++ b/llvm/test/Transforms/InstCombine/and-fcmp.ll
@@ -5044,11 +5044,9 @@ define i1 @isnormal_logical_select_0_fmf1(half %x) {
define i1 @and_fcmp_reassoc1(i1 %x, double %a, double %b) {
; CHECK-LABEL: @and_fcmp_reassoc1(
-; CHECK-NEXT: [[TMP1:%.*]] = fcmp ult double [[A:%.*]], [[B:%.*]]
-; CHECK-NEXT: [[CMP1:%.*]] = fcmp ugt double [[A]], [[B]]
+; CHECK-NEXT: [[TMP1:%.*]] = fcmp uno double [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[RETVAL:%.*]] = and i1 [[TMP1]], [[X:%.*]]
-; CHECK-NEXT: [[RETVAL1:%.*]] = and i1 [[RETVAL]], [[CMP1]]
-; CHECK-NEXT: ret i1 [[RETVAL1]]
+; CHECK-NEXT: ret i1 [[RETVAL]]
;
%cmp = fcmp ult double %a, %b
%cmp1 = fcmp ugt double %a, %b
@@ -5059,11 +5057,9 @@ define i1 @and_fcmp_reassoc1(i1 %x, double %a, double %b) {
define i1 @and_fcmp_reassoc2(i1 %x, double %a, double %b) {
; CHECK-LABEL: @and_fcmp_reassoc2(
-; CHECK-NEXT: [[TMP1:%.*]] = fcmp ult double [[A:%.*]], [[B:%.*]]
-; CHECK-NEXT: [[CMP1:%.*]] = fcmp ugt double [[A]], [[B]]
+; CHECK-NEXT: [[TMP1:%.*]] = fcmp uno double [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[RETVAL:%.*]] = and i1 [[X:%.*]], [[TMP1]]
-; CHECK-NEXT: [[RETVAL1:%.*]] = and i1 [[RETVAL]], [[CMP1]]
-; CHECK-NEXT: ret i1 [[RETVAL1]]
+; CHECK-NEXT: ret i1 [[RETVAL]]
;
%cmp = fcmp ult double %a, %b
%cmp1 = fcmp ugt double %a, %b
@@ -5074,11 +5070,9 @@ define i1 @and_fcmp_reassoc2(i1 %x, double %a, double %b) {
define i1 @and_fcmp_reassoc3(i1 %x, double %a, double %b) {
; CHECK-LABEL: @and_fcmp_reassoc3(
-; CHECK-NEXT: [[TMP1:%.*]] = fcmp ult double [[A:%.*]], [[B:%.*]]
-; CHECK-NEXT: [[CMP1:%.*]] = fcmp ugt double [[A]], [[B]]
+; CHECK-NEXT: [[TMP1:%.*]] = fcmp uno double [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[RETVAL:%.*]] = and i1 [[TMP1]], [[X:%.*]]
-; CHECK-NEXT: [[RETVAL1:%.*]] = and i1 [[CMP1]], [[RETVAL]]
-; CHECK-NEXT: ret i1 [[RETVAL1]]
+; CHECK-NEXT: ret i1 [[RETVAL]]
;
%cmp = fcmp ult double %a, %b
%cmp1 = fcmp ugt double %a, %b
@@ -5089,11 +5083,9 @@ define i1 @and_fcmp_reassoc3(i1 %x, double %a, double %b) {
define i1 @and_fcmp_reassoc4(i1 %x, double %a, double %b) {
; CHECK-LABEL: @and_fcmp_reassoc4(
-; CHECK-NEXT: [[TMP1:%.*]] = fcmp ult double [[A:%.*]], [[B:%.*]]
-; CHECK-NEXT: [[CMP1:%.*]] = fcmp ugt double [[A]], [[B]]
+; CHECK-NEXT: [[TMP1:%.*]] = fcmp uno double [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[RETVAL:%.*]] = and i1 [[X:%.*]], [[TMP1]]
-; CHECK-NEXT: [[RETVAL1:%.*]] = and i1 [[CMP1]], [[RETVAL]]
-; CHECK-NEXT: ret i1 [[RETVAL1]]
+; CHECK-NEXT: ret i1 [[RETVAL]]
;
%cmp = fcmp ult double %a, %b
%cmp1 = fcmp ugt double %a, %b
diff --git a/llvm/test/Transforms/InstCombine/or-fcmp.ll b/llvm/test/Transforms/InstCombine/or-fcmp.ll
index a2842f7a45f597..193fe4b5cc722f 100644
--- a/llvm/test/Transforms/InstCombine/or-fcmp.ll
+++ b/llvm/test/Transforms/InstCombine/or-fcmp.ll
@@ -54,7 +54,7 @@ define i1 @PR41069(double %a, double %b, double %c, double %d) {
; CHECK-LABEL: @PR41069(
; CHECK-NEXT: [[UNO1:%.*]] = fcmp uno double [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[TMP1:%.*]] = fcmp uno double [[D:%.*]], [[C:%.*]]
-; CHECK-NEXT: [[R:%.*]] = or i1 [[TMP1]], [[UNO1]]
+; CHECK-NEXT: [[R:%.*]] = or i1 [[UNO1]], [[TMP1]]
; CHECK-NEXT: ret i1 [[R]]
;
%uno1 = fcmp uno double %a, %b
@@ -87,7 +87,7 @@ define i1 @PR41069_commute(double %a, double %b, double %c, double %d) {
; CHECK-LABEL: @PR41069_commute(
; CHECK-NEXT: [[UNO1:%.*]] = fcmp uno double [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[TMP1:%.*]] = fcmp uno double [[D:%.*]], [[C:%.*]]
-; CHECK-NEXT: [[R:%.*]] = or i1 [[TMP1]], [[UNO1]]
+; CHECK-NEXT: [[R:%.*]] = or i1 [[UNO1]], [[TMP1]]
; CHECK-NEXT: ret i1 [[R]]
;
%uno1 = fcmp uno double %a, %b
@@ -4608,11 +4608,9 @@ define i1 @intersect_fmf_4(double %a, double %b) {
define i1 @or_fcmp_reassoc1(i1 %x, double %a, double %b) {
; CHECK-LABEL: @or_fcmp_reassoc1(
-; CHECK-NEXT: [[OR:%.*]] = fcmp olt double [[A:%.*]], [[B:%.*]]
-; CHECK-NEXT: [[CMP2:%.*]] = fcmp ogt double [[A]], [[B]]
+; CHECK-NEXT: [[OR:%.*]] = fcmp one double [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[RETVAL:%.*]] = or i1 [[OR]], [[CMP1:%.*]]
-; CHECK-NEXT: [[RETVAL1:%.*]] = or i1 [[RETVAL]], [[CMP2]]
-; CHECK-NEXT: ret i1 [[RETVAL1]]
+; CHECK-NEXT: ret i1 [[RETVAL]]
;
%cmp = fcmp olt double %a, %b
%cmp1 = fcmp ogt double %a, %b
@@ -4623,11 +4621,9 @@ define i1 @or_fcmp_reassoc1(i1 %x, double %a, double %b) {
define i1 @or_fcmp_reassoc2(i1 %x, double %a, double %b) {
; CHECK-LABEL: @or_fcmp_reassoc2(
-; CHECK-NEXT: [[TMP1:%.*]] = fcmp olt double [[A:%.*]], [[B:%.*]]
-; CHECK-NEXT: [[CMP1:%.*]] = fcmp ogt double [[A]], [[B]]
+; CHECK-NEXT: [[TMP1:%.*]] = fcmp one double [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[RETVAL:%.*]] = or i1 [[X:%.*]], [[TMP1]]
-; CHECK-NEXT: [[RETVAL1:%.*]] = or i1 [[RETVAL]], [[CMP1]]
-; CHECK-NEXT: ret i1 [[RETVAL1]]
+; CHECK-NEXT: ret i1 [[RETVAL]]
;
%cmp = fcmp olt double %a, %b
%cmp1 = fcmp ogt double %a, %b
@@ -4638,11 +4634,9 @@ define i1 @or_fcmp_reassoc2(i1 %x, double %a, double %b) {
define i1 @or_fcmp_reassoc3(i1 %x, double %a, double %b) {
; CHECK-LABEL: @or_fcmp_reassoc3(
-; CHECK-NEXT: [[TMP1:%.*]] = fcmp olt double [[A:%.*]], [[B:%.*]]
-; CHECK-NEXT: [[CMP1:%.*]] = fcmp ogt double [[A]], [[B]]
+; CHECK-NEXT: [[TMP1:%.*]] = fcmp one double [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[RETVAL:%.*]] = or i1 [[TMP1]], [[X:%.*]]
-; CHECK-NEXT: [[RETVAL1:%.*]] = or i1 [[CMP1]], [[RETVAL]]
-; CHECK-NEXT: ret i1 [[RETVAL1]]
+; CHECK-NEXT: ret i1 [[RETVAL]]
;
%cmp = fcmp olt double %a, %b
%cmp1 = fcmp ogt double %a, %b
@@ -4653,11 +4647,9 @@ define i1 @or_fcmp_reassoc3(i1 %x, double %a, double %b) {
define i1 @or_fcmp_reassoc4(i1 %x, double %a, double %b) {
; CHECK-LABEL: @or_fcmp_reassoc4(
-; CHECK-NEXT: [[TMP1:%.*]] = fcmp olt double [[A:%.*]], [[B:%.*]]
-; CHECK-NEXT: [[CMP1:%.*]] = fcmp ogt double [[A]], [[B]]
+; CHECK-NEXT: [[TMP1:%.*]] = fcmp one double [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[RETVAL:%.*]] = or i1 [[X:%.*]], [[TMP1]]
-; CHECK-NEXT: [[RETVAL1:%.*]] = or i1 [[CMP1]], [[RETVAL]]
-; CHECK-NEXT: ret i1 [[RETVAL1]]
+; CHECK-NEXT: ret i1 [[RETVAL]]
;
%cmp = fcmp olt double %a, %b
%cmp1 = fcmp ogt double %a, %b
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
nikic
force-pushed
the
instcombine-and-or-reassoc
branch
from
c23092a to
ec93e70
Compare
This was referenced Nov 14, 2024
dtcxzyw
approved these changes
Nov 14, 2024
There was a problem hiding this comment.
LGTM. Please wait for additional approval from other reviewers.
Missing fold: https://alive2.llvm.org/ce/z/wawsAb
I will post a patch later.
dtcxzyw
added a commit
that referenced
this pull request
Nov 19, 2024
) This patch generalizes #81027 to handle pattern `and/or (fcmp ord/uno X, 0), (fcmp pred fabs(X), Y)`. Alive2: https://alive2.llvm.org/ce/z/tsgUrz The correctness is straightforward because `fcmp ord/uno X, 0.0` is equivalent to `fcmp ord/uno fabs(X), 0.0`. We may generalize it to handle fneg as well. Address comment #116065 (review)
|
Ping for the second opinion :) |
arsenm
approved these changes
Nov 22, 2024
Comment on lines
1473
to
1475
| FastMathFlags FMF = LHS->getFastMathFlags(); | ||
| FMF &= RHS->getFastMathFlags(); | ||
| Builder.setFastMathFlags(FMF); |
There was a problem hiding this comment.
You can do the and inline now
Suggested change
| FastMathFlags FMF = LHS->getFastMathFlags(); | |
| FMF &= RHS->getFastMathFlags(); | |
| Builder.setFastMathFlags(FMF); | |
| Builder.setFastMathFlags(LHS->getFastMathFlags() & RHS->getFastMathFlags()); |
| IRBuilder<>::FastMathFlagGuard FMFG(Builder); | ||
| FastMathFlags FMF = LHS->getFastMathFlags(); | ||
| FMF &= RHS->getFastMathFlags(); | ||
| Builder.setFastMathFlags(FMF); | ||
| return Builder.CreateFCmp(PredL, LHS0, RHS0); |
There was a problem hiding this comment.
Really we ought to have FMF parameters for all the Create* functions instead of the flag guard boilerplate
We currently support simple reassociation for foldAndOrOfICmps(). Support the same for foldLogicOfFCmps() by going through the common foldBooleanAndOr() helper. This will also resolve the regression on llvm#112704, which is also due to missing reassoc support. I had to adjust one fold to add support for FMF flag preservation, otherwise there would be test regressions. There is a separate fold (reassociateFCmps) handling reassociation for *just* that specific case and it preserves FMF. Unfortuantely it's not rendered entirely redundant by this patch, because it handles one more level of reassociation as well.
nikic
force-pushed
the
instcombine-and-or-reassoc
branch
from
ec93e70 to
cbb6260
Compare
dtcxzyw
added a commit
that referenced
this pull request
Jan 6, 2025
In #116065, we pass `IsLogical` into `foldBooleanAndOr` when folding inner and/or ops. But it is always safe to treat them as bitwise if the outer ops are bitwise. Alive2: https://alive2.llvm.org/ce/z/hULrgH Closes #121701.
github-actions bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Jan 10, 2025
In llvm/llvm-project#116065, we pass `IsLogical` into `foldBooleanAndOr` when folding inner and/or ops. But it is always safe to treat them as bitwise if the outer ops are bitwise. Alive2: https://alive2.llvm.org/ce/z/hULrgH Closes llvm/llvm-project#121701.
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.
We currently support simple reassociation for foldAndOrOfICmps(). Support the same for foldLogicOfFCmps() by going through the common foldBooleanAndOr() helper.
This will also resolve the regression on #112704, which is also due to missing reassoc support.
I had to adjust one fold to add support for FMF flag preservation, otherwise there would be test regressions. There is a separate fold (reassociateFCmps) handling reassociation for just that specific case and it preserves FMF. Unfortuantely it's not rendered entirely redundant by this patch, because it handles one more level of reassociation as well.