-
Notifications
You must be signed in to change notification settings - Fork 14k
[Local] Verify opcodes match for all insts passed to mergeFlags (NFC). #141231
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
Conversation
@llvm/pr-subscribers-llvm-transforms Author: Florian Hahn (fhahn) ChangesThe logic for tracking flags relies on all instructions having the same opcode. Add an assert to check, as suggested in #140406. Full diff: https://github.com/llvm/llvm-project/pull/141231.diff 4 Files Affected:
diff --git a/llvm/include/llvm/Transforms/Utils/Local.h b/llvm/include/llvm/Transforms/Utils/Local.h
index fa26446f9492a..6162557496405 100644
--- a/llvm/include/llvm/Transforms/Utils/Local.h
+++ b/llvm/include/llvm/Transforms/Utils/Local.h
@@ -563,6 +563,13 @@ bool inferAttributesFromOthers(Function &F);
struct OverflowTracking {
bool HasNUW = true;
bool HasNSW = true;
+ bool IsDisjoint = true;
+
+#ifndef NDEBUG
+ /// Opcode of merged instructions. All instructions passed to mergeFlags must
+ /// have the same opcode.
+ std::optional<unsigned> Opcode;
+#endif
// Note: At the moment, users are responsible to manage AllKnownNonNegative
// and AllKnownNonZero manually. AllKnownNonNegative can be true in a case
diff --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp
index 7c7e0dcff0886..006a09b38bc71 100644
--- a/llvm/lib/Transforms/Scalar/LICM.cpp
+++ b/llvm/lib/Transforms/Scalar/LICM.cpp
@@ -2870,13 +2870,6 @@ static bool hoistBOAssociation(Instruction &I, Loop &L,
if (auto *I = dyn_cast<Instruction>(Inv))
I->setFastMathFlags(Intersect);
NewBO->setFastMathFlags(Intersect);
- } else if (Opcode == Instruction::Or) {
- bool Disjoint = cast<PossiblyDisjointInst>(BO)->isDisjoint() &&
- cast<PossiblyDisjointInst>(BO0)->isDisjoint();
- // If `Inv` was not constant-folded, a new Instruction has been created.
- if (auto *I = dyn_cast<PossiblyDisjointInst>(Inv))
- I->setIsDisjoint(Disjoint);
- cast<PossiblyDisjointInst>(NewBO)->setIsDisjoint(Disjoint);
} else {
OverflowTracking Flags;
Flags.AllKnownNonNegative = false;
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 4d168ce7cf591..fe1391a501b43 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -4364,10 +4364,19 @@ bool llvm::inferAttributesFromOthers(Function &F) {
}
void OverflowTracking::mergeFlags(Instruction &I) {
+#ifndef NDEBUG
+ if (Opcode)
+ assert(Opcode == I.getOpcode() &&
+ "can only use mergeFlags on instructions with matching opcodes");
+ else
+ Opcode = I.getOpcode();
+#endif
if (isa<OverflowingBinaryOperator>(&I)) {
HasNUW &= I.hasNoUnsignedWrap();
HasNSW &= I.hasNoSignedWrap();
}
+ if (auto *DisjointOp = dyn_cast<PossiblyDisjointInst>(&I))
+ IsDisjoint &= DisjointOp->isDisjoint();
}
void OverflowTracking::applyFlags(Instruction &I) {
@@ -4379,4 +4388,6 @@ void OverflowTracking::applyFlags(Instruction &I) {
if (HasNSW && (AllKnownNonNegative || HasNUW))
I.setHasNoSignedWrap();
}
+ if (auto *DisjointOp = dyn_cast<PossiblyDisjointInst>(&I))
+ DisjointOp->setIsDisjoint(IsDisjoint);
}
diff --git a/llvm/test/Transforms/Reassociate/or-disjoint.ll b/llvm/test/Transforms/Reassociate/or-disjoint.ll
index 777836ed98152..b060b94e01d69 100644
--- a/llvm/test/Transforms/Reassociate/or-disjoint.ll
+++ b/llvm/test/Transforms/Reassociate/or-disjoint.ll
@@ -4,8 +4,8 @@
define i16 @or_disjoint_both(i16 %a, i16 %b) {
; CHECK-LABEL: @or_disjoint_both(
-; CHECK-NEXT: [[OR_1:%.*]] = or i16 [[A:%.*]], 1
-; CHECK-NEXT: [[OR_2:%.*]] = or i16 [[OR_1]], [[B:%.*]]
+; CHECK-NEXT: [[OR_1:%.*]] = or disjoint i16 [[A:%.*]], 1
+; CHECK-NEXT: [[OR_2:%.*]] = or disjoint i16 [[OR_1]], [[B:%.*]]
; CHECK-NEXT: ret i16 [[OR_2]]
;
%or.1 = or disjoint i16 %b, %a
|
0222f2f
to
0ba4571
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…Flags (NFC). (#141231) The logic for tracking flags relies on all instructions having the same opcode. Add an assert to check, as suggested in llvm/llvm-project#140406. PR: llvm/llvm-project#141231
llvm#141231) The logic for tracking flags relies on all instructions having the same opcode. Add an assert to check, as suggested in llvm#140406. PR: llvm#141231
The logic for tracking flags relies on all instructions having the same opcode. Add an assert to check, as suggested in #140406.