From 526e456d7052655dc873e53c659c0e365179554b Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Wed, 15 Nov 2023 16:07:48 -0800 Subject: [PATCH 01/15] [IR] Add disjoint flag for Or instructions. This flag indicates that every bit is known to be zero in at least one of the inputs. This allows the Or to be treated as an Add since there is no possibility of a carry from any bit. If the flag is present and this property does not hold, the result is poison. This makes it easier to reverse the InstCombine transform that turns Add into Or. I will start a discourse thread as well. --- llvm/docs/LangRef.rst | 7 +++++++ llvm/include/llvm/AsmParser/LLToken.h | 1 + llvm/include/llvm/Bitcode/LLVMBitCodes.h | 4 ++++ llvm/include/llvm/IR/InstrTypes.h | 18 ++++++++++++++++- llvm/include/llvm/IR/Instruction.h | 8 ++++++++ llvm/lib/AsmParser/LLLexer.cpp | 1 + llvm/lib/AsmParser/LLParser.cpp | 8 +++++++- llvm/lib/Bitcode/Reader/BitcodeReader.cpp | 3 +++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp | 3 +++ llvm/lib/IR/AsmWriter.cpp | 4 ++++ llvm/lib/IR/Instruction.cpp | 23 ++++++++++++++++++++++ llvm/test/Assembler/flags.ll | 5 +++++ llvm/test/Bitcode/flags.ll | 4 ++++ llvm/test/Transforms/InstCombine/freeze.ll | 11 +++++++++++ 14 files changed, 98 insertions(+), 2 deletions(-) diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index bc1eab1e0b7a0..a4ea477870448 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -9981,6 +9981,7 @@ Syntax: :: = or , ; yields ty:result + = or disjoint , ; yields ty:result Overview: """"""""" @@ -10012,6 +10013,12 @@ The truth table used for the '``or``' instruction is: | 1 | 1 | 1 | +-----+-----+-----+ +``disjoint`` means every bit is known to be zero in at least one of the inputs. +This allows the Or to be treated as an Add since no carry can occur from any +bit. If the disjoint keyword is present, the result value of the ``or`` is a +:ref:`poison value ` if both inputs have a one in any bit +position. For vectors, only the element containing the bit is poison. + Example: """""""" diff --git a/llvm/include/llvm/AsmParser/LLToken.h b/llvm/include/llvm/AsmParser/LLToken.h index c9dcd29b31955..f4b12938590fe 100644 --- a/llvm/include/llvm/AsmParser/LLToken.h +++ b/llvm/include/llvm/AsmParser/LLToken.h @@ -109,6 +109,7 @@ enum Kind { kw_nuw, kw_nsw, kw_exact, + kw_disjoint, kw_inbounds, kw_nneg, kw_inrange, diff --git a/llvm/include/llvm/Bitcode/LLVMBitCodes.h b/llvm/include/llvm/Bitcode/LLVMBitCodes.h index 9fa70c0671ef3..99a41fa107d08 100644 --- a/llvm/include/llvm/Bitcode/LLVMBitCodes.h +++ b/llvm/include/llvm/Bitcode/LLVMBitCodes.h @@ -512,6 +512,10 @@ enum PossiblyNonNegInstOptionalFlags { PNNI_NON_NEG = 0 }; /// PossiblyExactOperator's SubclassOptionalData contents. enum PossiblyExactOperatorOptionalFlags { PEO_EXACT = 0 }; +/// PossiblyDisjointInstOptionalFlags - Flags for serializing +/// PossiblyDisjointInst's SubclassOptionalData contents. +enum PossiblyDisjointInstOptionalFlags { PDI_DISJOINT = 0 }; + /// Encoded AtomicOrdering values. enum AtomicOrderingCodes { ORDERING_NOTATOMIC = 0, diff --git a/llvm/include/llvm/IR/InstrTypes.h b/llvm/include/llvm/IR/InstrTypes.h index fc5e228168a05..99145ab9acd7f 100644 --- a/llvm/include/llvm/IR/InstrTypes.h +++ b/llvm/include/llvm/IR/InstrTypes.h @@ -415,6 +415,22 @@ struct OperandTraits : DEFINE_TRANSPARENT_OPERAND_ACCESSORS(BinaryOperator, Value) +/// A or instruction, which can be marked as "disjoint", indicating that the +/// inputs don't have a 1 in the same bit position. Meaning this instruction +/// can also be treated as an add. +class PossiblyDisjointInst : public BinaryOperator { +public: + enum { IsDisjoint = (1 << 0) }; + + static bool classof(const Instruction *I) { + return I->getOpcode() == Instruction::Or; + } + + static bool classof(const Value *V) { + return isa(V) && classof(cast(V)); + } +}; + //===----------------------------------------------------------------------===// // CastInst Class //===----------------------------------------------------------------------===// @@ -1085,7 +1101,7 @@ class CmpInst : public Instruction { } }; -// FIXME: these are redundant if CmpInst < BinaryOperator +// FIXME: these are redundant if CmpInst < ninaryOperator template <> struct OperandTraits : public FixedNumOperandTraits { }; diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h index 58fc32237367d..ba5fc35d0d408 100644 --- a/llvm/include/llvm/IR/Instruction.h +++ b/llvm/include/llvm/IR/Instruction.h @@ -448,6 +448,11 @@ class Instruction : public User, /// which supports this flag. See LangRef.html for the meaning of this flag. void setIsExact(bool b = true); + /// Set or clear the disjoint flag on this instruction, which must be an + /// operator which supports this flag. See LangRef.html for the meaning of + /// this flag. + void setIsDisjoint(bool b = true); + /// Set or clear the nneg flag on this instruction, which must be a zext /// instruction. void setNonNeg(bool b = true); @@ -500,6 +505,9 @@ class Instruction : public User, /// Determine whether the exact flag is set. bool isExact() const LLVM_READONLY; + /// Determine whether the disjoint flag is set. + bool isDisjoint() const LLVM_READONLY; + /// Set or clear all fast-math-flags on this instruction, which must be an /// operator which supports this flag. See LangRef.html for the meaning of /// this flag. diff --git a/llvm/lib/AsmParser/LLLexer.cpp b/llvm/lib/AsmParser/LLLexer.cpp index da9e9f4a3c983..854aa9cca2c5e 100644 --- a/llvm/lib/AsmParser/LLLexer.cpp +++ b/llvm/lib/AsmParser/LLLexer.cpp @@ -564,6 +564,7 @@ lltok::Kind LLLexer::LexIdentifier() { KEYWORD(nuw); KEYWORD(nsw); KEYWORD(exact); + KEYWORD(disjoint); KEYWORD(inbounds); KEYWORD(nneg); KEYWORD(inrange); diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp index f9df70fb6fc09..0c170d8da9b73 100644 --- a/llvm/lib/AsmParser/LLParser.cpp +++ b/llvm/lib/AsmParser/LLParser.cpp @@ -6368,8 +6368,14 @@ int LLParser::parseInstruction(Instruction *&Inst, BasicBlock *BB, case lltok::kw_srem: return parseArithmetic(Inst, PFS, KeywordVal, /*IsFP*/ false); + case lltok::kw_or: { + bool Disjoint = EatIfPresent(lltok::kw_disjoint); + if (parseLogical(Inst, PFS, KeywordVal)) + return true; + if (Disjoint) cast(Inst)->setIsDisjoint(true); + return false; + } case lltok::kw_and: - case lltok::kw_or: case lltok::kw_xor: return parseLogical(Inst, PFS, KeywordVal); case lltok::kw_icmp: diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp index 76431e883b8d9..e5aaa56f575c3 100644 --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp @@ -4870,6 +4870,9 @@ Error BitcodeReader::parseFunctionBody(Function *F) { Opc == Instruction::AShr) { if (Record[OpNum] & (1 << bitc::PEO_EXACT)) cast(I)->setIsExact(true); + } else if (Opc == Instruction::Or) { + if (Record[OpNum] & (1 << bitc::PDI_DISJOINT)) + cast(I)->setIsDisjoint(true); } else if (isa(I)) { FastMathFlags FMF = getDecodedFastMathFlags(Record[OpNum]); if (FMF.any()) diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp index d16b5c7781c24..135801a5c61c4 100644 --- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -1541,6 +1541,9 @@ static uint64_t getOptimizationFlags(const Value *V) { } else if (const auto *PEO = dyn_cast(V)) { if (PEO->isExact()) Flags |= 1 << bitc::PEO_EXACT; + } else if (const auto *PDI = dyn_cast(V)) { + if (PDI->isDisjoint()) + Flags |= 1 << bitc::PDI_DISJOINT; } else if (const auto *FPMO = dyn_cast(V)) { if (FPMO->hasAllowReassoc()) Flags |= bitc::AllowReassoc; diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp index 6d66b34423949..688f1d7e078ea 100644 --- a/llvm/lib/IR/AsmWriter.cpp +++ b/llvm/lib/IR/AsmWriter.cpp @@ -1355,6 +1355,10 @@ static void WriteOptimizationInfo(raw_ostream &Out, const User *U) { dyn_cast(U)) { if (Div->isExact()) Out << " exact"; + } else if (const PossiblyDisjointInst *PDI = + dyn_cast(U)) { + if (PDI->isDisjoint()) + Out << " disjoint"; } else if (const GEPOperator *GEP = dyn_cast(U)) { if (GEP->isInBounds()) Out << " inbounds"; diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp index 7449692f05d7b..fcf79f6cd8c46 100644 --- a/llvm/lib/IR/Instruction.cpp +++ b/llvm/lib/IR/Instruction.cpp @@ -317,6 +317,12 @@ void Instruction::setIsExact(bool b) { cast(this)->setIsExact(b); } +void Instruction::setIsDisjoint(bool b) { + assert(isa(this) && "Must be or"); + SubclassOptionalData = (SubclassOptionalData & ~PossiblyDisjointInst::IsDisjoint) | + (b * PossiblyDisjointInst::IsDisjoint); +} + void Instruction::setNonNeg(bool b) { assert(isa(this) && "Must be zext"); SubclassOptionalData = (SubclassOptionalData & ~PossiblyNonNegInst::NonNeg) | @@ -357,6 +363,10 @@ void Instruction::dropPoisonGeneratingFlags() { cast(this)->setIsExact(false); break; + case Instruction::Or: + cast(this)->setIsDisjoint(false); + break; + case Instruction::GetElementPtr: cast(this)->setIsInBounds(false); break; @@ -419,6 +429,11 @@ bool Instruction::isExact() const { return cast(this)->isExact(); } +bool Instruction::isDisjoint() const { + assert(isa(this) && "Must be or"); + return (SubclassOptionalData & PossiblyDisjointInst::IsDisjoint) != 0; +} + void Instruction::setFast(bool B) { assert(isa(this) && "setting fast-math flag on invalid op"); cast(this)->setFast(B); @@ -532,6 +547,10 @@ void Instruction::copyIRFlags(const Value *V, bool IncludeWrapFlags) { if (isa(this)) setIsExact(PE->isExact()); + if (auto *PD = dyn_cast(V)) + if (isa(this)) + setIsDisjoint(PD->isDisjoint()); + // Copy the fast-math flags. if (auto *FP = dyn_cast(V)) if (isa(this)) @@ -558,6 +577,10 @@ void Instruction::andIRFlags(const Value *V) { if (isa(this)) setIsExact(isExact() && PE->isExact()); + if (auto *PE = dyn_cast(V)) + if (isa(this)) + setIsDisjoint(isDisjoint() && PE->isDisjoint()); + if (auto *FP = dyn_cast(V)) { if (isa(this)) { FastMathFlags FM = getFastMathFlags(); diff --git a/llvm/test/Assembler/flags.ll b/llvm/test/Assembler/flags.ll index 6ab5e1bfb9c4f..04bddd02f50c8 100644 --- a/llvm/test/Assembler/flags.ll +++ b/llvm/test/Assembler/flags.ll @@ -256,3 +256,8 @@ define i64 @test_zext(i32 %a) { ret i64 %res } +define i64 @test_or(i64 %a, i64 %b) { +; CHECK: %res = or disjoint i64 %a, %b + %res = or disjoint i64 %a, %b + ret i64 %res +} diff --git a/llvm/test/Bitcode/flags.ll b/llvm/test/Bitcode/flags.ll index a6e368b7e7632..e3fc827d865d7 100644 --- a/llvm/test/Bitcode/flags.ll +++ b/llvm/test/Bitcode/flags.ll @@ -18,6 +18,8 @@ second: ; preds = %first %z = add i32 %a, 0 ; [#uses=0] %hh = zext nneg i32 %a to i64 %ll = zext i32 %s to i64 + %jj = or disjoint i32 %a, 0 + %oo = or i32 %a, 0 unreachable first: ; preds = %entry @@ -28,5 +30,7 @@ first: ; preds = %entry %zz = add i32 %a, 0 ; [#uses=0] %kk = zext nneg i32 %a to i64 %rr = zext i32 %ss to i64 + %mm = or disjoint i32 %a, 0 + %nn = or i32 %a, 0 br label %second } diff --git a/llvm/test/Transforms/InstCombine/freeze.ll b/llvm/test/Transforms/InstCombine/freeze.ll index dd9272b4b35f1..da59101d5710c 100644 --- a/llvm/test/Transforms/InstCombine/freeze.ll +++ b/llvm/test/Transforms/InstCombine/freeze.ll @@ -1127,6 +1127,17 @@ define i32 @freeze_zext_nneg(i8 %x) { ret i32 %fr } +define i32 @propagate_drop_flags_or(i32 %arg) { +; CHECK-LABEL: @propagate_drop_flags_or( +; CHECK-NEXT: [[ARG_FR:%.*]] = freeze i32 [[ARG:%.*]] +; CHECK-NEXT: [[V1:%.*]] = or i32 [[ARG_FR]], 2 +; CHECK-NEXT: ret i32 [[V1]] +; + %v1 = or disjoint i32 %arg, 2 + %v1.fr = freeze i32 %v1 + ret i32 %v1.fr +} + !0 = !{} !1 = !{i64 4} !2 = !{i32 0, i32 100} From 1697ccd00611bd563500e368d00e5fe803f9a832 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Thu, 16 Nov 2023 16:10:51 -0800 Subject: [PATCH 02/15] clang-format --- llvm/lib/AsmParser/LLParser.cpp | 3 ++- llvm/lib/Bitcode/Reader/BitcodeReader.cpp | 1 - llvm/lib/IR/AsmWriter.cpp | 2 +- llvm/lib/IR/Instruction.cpp | 5 +++-- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp index 0c170d8da9b73..83147e871fa0e 100644 --- a/llvm/lib/AsmParser/LLParser.cpp +++ b/llvm/lib/AsmParser/LLParser.cpp @@ -6372,7 +6372,8 @@ int LLParser::parseInstruction(Instruction *&Inst, BasicBlock *BB, bool Disjoint = EatIfPresent(lltok::kw_disjoint); if (parseLogical(Inst, PFS, KeywordVal)) return true; - if (Disjoint) cast(Inst)->setIsDisjoint(true); + if (Disjoint) + cast(Inst)->setIsDisjoint(true); return false; } case lltok::kw_and: diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp index e5aaa56f575c3..3e21a1a072834 100644 --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp @@ -4878,7 +4878,6 @@ Error BitcodeReader::parseFunctionBody(Function *F) { if (FMF.any()) I->setFastMathFlags(FMF); } - } break; } diff --git a/llvm/lib/IR/AsmWriter.cpp b/llvm/lib/IR/AsmWriter.cpp index 688f1d7e078ea..8ea33c71dc8f7 100644 --- a/llvm/lib/IR/AsmWriter.cpp +++ b/llvm/lib/IR/AsmWriter.cpp @@ -1356,7 +1356,7 @@ static void WriteOptimizationInfo(raw_ostream &Out, const User *U) { if (Div->isExact()) Out << " exact"; } else if (const PossiblyDisjointInst *PDI = - dyn_cast(U)) { + dyn_cast(U)) { if (PDI->isDisjoint()) Out << " disjoint"; } else if (const GEPOperator *GEP = dyn_cast(U)) { diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp index fcf79f6cd8c46..4163a179d589f 100644 --- a/llvm/lib/IR/Instruction.cpp +++ b/llvm/lib/IR/Instruction.cpp @@ -319,8 +319,9 @@ void Instruction::setIsExact(bool b) { void Instruction::setIsDisjoint(bool b) { assert(isa(this) && "Must be or"); - SubclassOptionalData = (SubclassOptionalData & ~PossiblyDisjointInst::IsDisjoint) | - (b * PossiblyDisjointInst::IsDisjoint); + SubclassOptionalData = + (SubclassOptionalData & ~PossiblyDisjointInst::IsDisjoint) | + (b * PossiblyDisjointInst::IsDisjoint); } void Instruction::setNonNeg(bool b) { From 97f1e05cc6d227118b82b35f4abe1cd2935d2791 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Fri, 17 Nov 2023 10:16:23 -0800 Subject: [PATCH 03/15] fixup! fix dropped character --- llvm/include/llvm/IR/InstrTypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/include/llvm/IR/InstrTypes.h b/llvm/include/llvm/IR/InstrTypes.h index 99145ab9acd7f..f77c7cf67fb33 100644 --- a/llvm/include/llvm/IR/InstrTypes.h +++ b/llvm/include/llvm/IR/InstrTypes.h @@ -1101,7 +1101,7 @@ class CmpInst : public Instruction { } }; -// FIXME: these are redundant if CmpInst < ninaryOperator +// FIXME: these are redundant if CmpInst < BinaryOperator template <> struct OperandTraits : public FixedNumOperandTraits { }; From bcb6398722473538fd67f8224c6e241b80baffde Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Fri, 17 Nov 2023 10:19:54 -0800 Subject: [PATCH 04/15] fixup! Add test for SimplifyCFG preserving and dropping the flag. --- llvm/test/Transforms/SimplifyCFG/HoistCode.ll | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/llvm/test/Transforms/SimplifyCFG/HoistCode.ll b/llvm/test/Transforms/SimplifyCFG/HoistCode.ll index 08cf6cd5be80c..a081eddfc4566 100644 --- a/llvm/test/Transforms/SimplifyCFG/HoistCode.ll +++ b/llvm/test/Transforms/SimplifyCFG/HoistCode.ll @@ -124,3 +124,33 @@ F: %z2 = zext i8 %x to i32 ret i32 %z2 } + +define i32 @hoist_or_flags_preserve(i1 %C, i32 %x, i32 %y) { +; CHECK-LABEL: @hoist_or_flags_preserve( +; CHECK-NEXT: common.ret: +; CHECK-NEXT: [[Z1:%.*]] = or disjoint i32 [[X:%.*]], [[Y:%.*]] +; CHECK-NEXT: ret i32 [[Z1]] +; + br i1 %C, label %T, label %F +T: + %z1 = or disjoint i32 %x, %y + ret i32 %z1 +F: + %z2 = or disjoint i32 %x, %y + ret i32 %z2 +} + +define i32 @hoist_or_flags_drop(i1 %C, i32 %x, i32 %y) { +; CHECK-LABEL: @hoist_or_flags_drop( +; CHECK-NEXT: common.ret: +; CHECK-NEXT: [[Z1:%.*]] = or i32 [[X:%.*]], [[Y:%.*]] +; CHECK-NEXT: ret i32 [[Z1]] +; + br i1 %C, label %T, label %F +T: + %z1 = or i32 %x, %y + ret i32 %z1 +F: + %z2 = or disjoint i32 %x, %y + ret i32 %z2 +} From e47fc47c063b6de4a8f33d303b9e9f7f1d4ac4dd Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Fri, 17 Nov 2023 10:24:13 -0800 Subject: [PATCH 05/15] fixup! clear disjoint flag in SimplifyDemandedBits. --- .../InstCombine/InstCombineSimplifyDemanded.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp index 69b95c0b35c68..012fce4e3ad66 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp @@ -231,8 +231,11 @@ Value *InstCombinerImpl::SimplifyDemandedUseBits(Value *V, APInt DemandedMask, // If either the LHS or the RHS are One, the result is One. if (SimplifyDemandedBits(I, 1, DemandedMask, RHSKnown, Depth + 1) || SimplifyDemandedBits(I, 0, DemandedMask & ~RHSKnown.One, LHSKnown, - Depth + 1)) + Depth + 1)) { + // Disjoint flag may not longer hold. + I->dropPoisonGeneratingFlags(); return I; + } assert(!RHSKnown.hasConflict() && "Bits known to be one AND zero?"); assert(!LHSKnown.hasConflict() && "Bits known to be one AND zero?"); @@ -252,8 +255,11 @@ Value *InstCombinerImpl::SimplifyDemandedUseBits(Value *V, APInt DemandedMask, return I->getOperand(1); // If the RHS is a constant, see if we can simplify it. - if (ShrinkDemandedConstant(I, 1, DemandedMask)) + if (ShrinkDemandedConstant(I, 1, DemandedMask)) { + // Disjoint flag may not longer hold. + I->dropPoisonGeneratingFlags(); return I; + } break; } From ae9087333889a02942ae60fff0abddd5b9264140 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Fri, 17 Nov 2023 10:50:55 -0800 Subject: [PATCH 06/15] fixup! Address LangRef feedback --- llvm/docs/LangRef.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index a4ea477870448..ef8e305790880 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -10016,7 +10016,7 @@ The truth table used for the '``or``' instruction is: ``disjoint`` means every bit is known to be zero in at least one of the inputs. This allows the Or to be treated as an Add since no carry can occur from any bit. If the disjoint keyword is present, the result value of the ``or`` is a -:ref:`poison value ` if both inputs have a one in any bit +:ref:`poison value ` if both inputs have a one in the same bit position. For vectors, only the element containing the bit is poison. Example: From 1ec8faaf40dc012b123b0e2f6432a4f027a49f7a Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Mon, 20 Nov 2023 10:39:19 -0800 Subject: [PATCH 07/15] fixup! address review comments --- llvm/include/llvm/IR/InstrTypes.h | 2 +- .../Transforms/InstCombine/InstCombineSimplifyDemanded.cpp | 5 +---- llvm/test/Transforms/InstCombine/or.ll | 7 +++++++ 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/llvm/include/llvm/IR/InstrTypes.h b/llvm/include/llvm/IR/InstrTypes.h index f77c7cf67fb33..3aedb8cd74a2c 100644 --- a/llvm/include/llvm/IR/InstrTypes.h +++ b/llvm/include/llvm/IR/InstrTypes.h @@ -415,7 +415,7 @@ struct OperandTraits : DEFINE_TRANSPARENT_OPERAND_ACCESSORS(BinaryOperator, Value) -/// A or instruction, which can be marked as "disjoint", indicating that the +/// An or instruction, which can be marked as "disjoint", indicating that the /// inputs don't have a 1 in the same bit position. Meaning this instruction /// can also be treated as an add. class PossiblyDisjointInst : public BinaryOperator { diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp index 012fce4e3ad66..fa076098d63cd 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp @@ -255,11 +255,8 @@ Value *InstCombinerImpl::SimplifyDemandedUseBits(Value *V, APInt DemandedMask, return I->getOperand(1); // If the RHS is a constant, see if we can simplify it. - if (ShrinkDemandedConstant(I, 1, DemandedMask)) { - // Disjoint flag may not longer hold. - I->dropPoisonGeneratingFlags(); + if (ShrinkDemandedConstant(I, 1, DemandedMask)) return I; - } break; } diff --git a/llvm/test/Transforms/InstCombine/or.ll b/llvm/test/Transforms/InstCombine/or.ll index fd53783a06f9d..3ccb25a2869cc 100644 --- a/llvm/test/Transforms/InstCombine/or.ll +++ b/llvm/test/Transforms/InstCombine/or.ll @@ -1576,3 +1576,10 @@ define <4 x i1> @and_or_not_or_logical_vec(<4 x i32> %ap, <4 x i32> %bp) { %Z = or <4 x i1> %X, %Y ret <4 x i1> %Z } + +; Make sure SimplifyDemandedBits drops the disjoint flag. +define i8 @drop_disjoint(i8 %x) { + %a = and i8 %x, -2 + %b = or disjoint i8 %a, 1 + ret i8 %b +} From 0ab2d7110d8256e6a3f6239e9377e5a2e6e39bb9 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Mon, 20 Nov 2023 13:15:38 -0800 Subject: [PATCH 08/15] fixup! add CHECK lines. --- llvm/test/Transforms/InstCombine/or.ll | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/llvm/test/Transforms/InstCombine/or.ll b/llvm/test/Transforms/InstCombine/or.ll index 3ccb25a2869cc..642a0282e5f7c 100644 --- a/llvm/test/Transforms/InstCombine/or.ll +++ b/llvm/test/Transforms/InstCombine/or.ll @@ -1579,6 +1579,10 @@ define <4 x i1> @and_or_not_or_logical_vec(<4 x i32> %ap, <4 x i32> %bp) { ; Make sure SimplifyDemandedBits drops the disjoint flag. define i8 @drop_disjoint(i8 %x) { +; CHECK-LABEL: @drop_disjoint( +; CHECK-NEXT: [[B:%.*]] = or i8 [[X:%.*]], 1 +; CHECK-NEXT: ret i8 [[B]] +; %a = and i8 %x, -2 %b = or disjoint i8 %a, 1 ret i8 %b From 93bc06edb7ab0e8d3b24aff378c43c8246ee01af Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Mon, 20 Nov 2023 18:14:47 -0800 Subject: [PATCH 09/15] fixup! try to improve the langref description --- llvm/docs/LangRef.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index ef8e305790880..37cdaced7d6cb 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -10013,11 +10013,11 @@ The truth table used for the '``or``' instruction is: | 1 | 1 | 1 | +-----+-----+-----+ -``disjoint`` means every bit is known to be zero in at least one of the inputs. -This allows the Or to be treated as an Add since no carry can occur from any -bit. If the disjoint keyword is present, the result value of the ``or`` is a -:ref:`poison value ` if both inputs have a one in the same bit -position. For vectors, only the element containing the bit is poison. +``disjoint`` means for each bit, that bit is known to be zero in at least one of +the inputs. This allows the Or to be treated as an Add since no carry can occur +from any bit. If the disjoint keyword is present, the result value of the ``or`` +is a :ref:`poison value ` if both inputs have a one in the same +bit position. For vectors, only the element containing the bit is poison. Example: """""""" From 73357382e7e0af5567945364da3d91226eb8430c Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Mon, 20 Nov 2023 18:36:22 -0800 Subject: [PATCH 10/15] fixup! refine LangRef again. --- llvm/docs/LangRef.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index 37cdaced7d6cb..ce68540da9a14 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -10013,11 +10013,11 @@ The truth table used for the '``or``' instruction is: | 1 | 1 | 1 | +-----+-----+-----+ -``disjoint`` means for each bit, that bit is known to be zero in at least one of -the inputs. This allows the Or to be treated as an Add since no carry can occur -from any bit. If the disjoint keyword is present, the result value of the ``or`` -is a :ref:`poison value ` if both inputs have a one in the same -bit position. For vectors, only the element containing the bit is poison. +``disjoint`` means for each bit, that bit is zero in at least one of the inputs. +This allows the Or to be treated as an Add since no carry can occur from any +bit. If the disjoint keyword is present, the result value of the ``or`` is a +:ref:`poison value ` if both inputs have a one in the same bit +position. For vectors, only the element containing the bit is poison. Example: """""""" From 3be62d27b678e7c866eab38fd7ce84f8c17fcfe4 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Wed, 22 Nov 2023 10:33:34 -0800 Subject: [PATCH 11/15] fixup! Add to Bitcode/compatibility.ll --- llvm/test/Bitcode/compatibility.ll | 3 +++ 1 file changed, 3 insertions(+) diff --git a/llvm/test/Bitcode/compatibility.ll b/llvm/test/Bitcode/compatibility.ll index 8170f18879aaf..6b65d81792370 100644 --- a/llvm/test/Bitcode/compatibility.ll +++ b/llvm/test/Bitcode/compatibility.ll @@ -1359,6 +1359,9 @@ define void @instructions.bitwise_binops(i8 %op1, i8 %op2) { xor i8 %op1, %op2 ; CHECK: xor i8 %op1, %op2 + ; disjoint + or disjoint i8 %op1, %op2 + ret void } From 850017e72a999405a1d770c8ee4182f5d7c93c30 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Wed, 22 Nov 2023 10:38:09 -0800 Subject: [PATCH 12/15] Add setIsDisjoint/isDisjoint to PossiblyDisjointInst. --- llvm/include/llvm/IR/InstrTypes.h | 11 +++++++++++ llvm/lib/AsmParser/LLParser.cpp | 2 +- llvm/lib/IR/Instruction.cpp | 8 ++------ 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/llvm/include/llvm/IR/InstrTypes.h b/llvm/include/llvm/IR/InstrTypes.h index 3aedb8cd74a2c..062b748ef0e9f 100644 --- a/llvm/include/llvm/IR/InstrTypes.h +++ b/llvm/include/llvm/IR/InstrTypes.h @@ -422,6 +422,17 @@ class PossiblyDisjointInst : public BinaryOperator { public: enum { IsDisjoint = (1 << 0) }; +private: + friend class Instruction; + + void setIsDisjoint(bool B) { + SubclassOptionalData = + (SubclassOptionalData & ~IsDisjoint) | (B * IsDisjoint); + } + +public: + bool isDisjoint() const { return SubclassOptionalData & IsDisjoint; } + static bool classof(const Instruction *I) { return I->getOpcode() == Instruction::Or; } diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp index 83147e871fa0e..59f4c3587287a 100644 --- a/llvm/lib/AsmParser/LLParser.cpp +++ b/llvm/lib/AsmParser/LLParser.cpp @@ -6373,7 +6373,7 @@ int LLParser::parseInstruction(Instruction *&Inst, BasicBlock *BB, if (parseLogical(Inst, PFS, KeywordVal)) return true; if (Disjoint) - cast(Inst)->setIsDisjoint(true); + cast(Inst)->setIsDisjoint(true); return false; } case lltok::kw_and: diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp index 4163a179d589f..d24a35e2e945d 100644 --- a/llvm/lib/IR/Instruction.cpp +++ b/llvm/lib/IR/Instruction.cpp @@ -318,10 +318,7 @@ void Instruction::setIsExact(bool b) { } void Instruction::setIsDisjoint(bool b) { - assert(isa(this) && "Must be or"); - SubclassOptionalData = - (SubclassOptionalData & ~PossiblyDisjointInst::IsDisjoint) | - (b * PossiblyDisjointInst::IsDisjoint); + cast(this)->setIsDisjoint(b); } void Instruction::setNonNeg(bool b) { @@ -431,8 +428,7 @@ bool Instruction::isExact() const { } bool Instruction::isDisjoint() const { - assert(isa(this) && "Must be or"); - return (SubclassOptionalData & PossiblyDisjointInst::IsDisjoint) != 0; + return cast(this)->isDisjoint(); } void Instruction::setFast(bool B) { From e5517b3c3b3fc3bcc9f9148216c96d93bdef5bcb Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Wed, 22 Nov 2023 11:29:39 -0800 Subject: [PATCH 13/15] fixup! add CHECK line to compatibility.ll --- llvm/test/Bitcode/compatibility.ll | 1 + 1 file changed, 1 insertion(+) diff --git a/llvm/test/Bitcode/compatibility.ll b/llvm/test/Bitcode/compatibility.ll index 6b65d81792370..483024a250da0 100644 --- a/llvm/test/Bitcode/compatibility.ll +++ b/llvm/test/Bitcode/compatibility.ll @@ -1361,6 +1361,7 @@ define void @instructions.bitwise_binops(i8 %op1, i8 %op2) { ; disjoint or disjoint i8 %op1, %op2 + ; CHECK: or disjoint i8 %op1, %op2 ret void } From 808d64d6270e106638ba8ad654b5e912677a5953 Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Wed, 22 Nov 2023 11:44:05 -0800 Subject: [PATCH 14/15] fixup! Remove Instruction::setIsDisjoint/isDisjoint --- llvm/include/llvm/IR/InstrTypes.h | 4 ---- llvm/include/llvm/IR/Instruction.h | 8 -------- llvm/lib/AsmParser/LLParser.cpp | 2 +- llvm/lib/Bitcode/Reader/BitcodeReader.cpp | 2 +- llvm/lib/IR/Instruction.cpp | 20 ++++++-------------- 5 files changed, 8 insertions(+), 28 deletions(-) diff --git a/llvm/include/llvm/IR/InstrTypes.h b/llvm/include/llvm/IR/InstrTypes.h index 062b748ef0e9f..ddae3e4f43f48 100644 --- a/llvm/include/llvm/IR/InstrTypes.h +++ b/llvm/include/llvm/IR/InstrTypes.h @@ -422,15 +422,11 @@ class PossiblyDisjointInst : public BinaryOperator { public: enum { IsDisjoint = (1 << 0) }; -private: - friend class Instruction; - void setIsDisjoint(bool B) { SubclassOptionalData = (SubclassOptionalData & ~IsDisjoint) | (B * IsDisjoint); } -public: bool isDisjoint() const { return SubclassOptionalData & IsDisjoint; } static bool classof(const Instruction *I) { diff --git a/llvm/include/llvm/IR/Instruction.h b/llvm/include/llvm/IR/Instruction.h index ba5fc35d0d408..58fc32237367d 100644 --- a/llvm/include/llvm/IR/Instruction.h +++ b/llvm/include/llvm/IR/Instruction.h @@ -448,11 +448,6 @@ class Instruction : public User, /// which supports this flag. See LangRef.html for the meaning of this flag. void setIsExact(bool b = true); - /// Set or clear the disjoint flag on this instruction, which must be an - /// operator which supports this flag. See LangRef.html for the meaning of - /// this flag. - void setIsDisjoint(bool b = true); - /// Set or clear the nneg flag on this instruction, which must be a zext /// instruction. void setNonNeg(bool b = true); @@ -505,9 +500,6 @@ class Instruction : public User, /// Determine whether the exact flag is set. bool isExact() const LLVM_READONLY; - /// Determine whether the disjoint flag is set. - bool isDisjoint() const LLVM_READONLY; - /// Set or clear all fast-math-flags on this instruction, which must be an /// operator which supports this flag. See LangRef.html for the meaning of /// this flag. diff --git a/llvm/lib/AsmParser/LLParser.cpp b/llvm/lib/AsmParser/LLParser.cpp index 59f4c3587287a..83147e871fa0e 100644 --- a/llvm/lib/AsmParser/LLParser.cpp +++ b/llvm/lib/AsmParser/LLParser.cpp @@ -6373,7 +6373,7 @@ int LLParser::parseInstruction(Instruction *&Inst, BasicBlock *BB, if (parseLogical(Inst, PFS, KeywordVal)) return true; if (Disjoint) - cast(Inst)->setIsDisjoint(true); + cast(Inst)->setIsDisjoint(true); return false; } case lltok::kw_and: diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp index 3e21a1a072834..3c713d5b096df 100644 --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp @@ -4872,7 +4872,7 @@ Error BitcodeReader::parseFunctionBody(Function *F) { cast(I)->setIsExact(true); } else if (Opc == Instruction::Or) { if (Record[OpNum] & (1 << bitc::PDI_DISJOINT)) - cast(I)->setIsDisjoint(true); + cast(I)->setIsDisjoint(true); } else if (isa(I)) { FastMathFlags FMF = getDecodedFastMathFlags(Record[OpNum]); if (FMF.any()) diff --git a/llvm/lib/IR/Instruction.cpp b/llvm/lib/IR/Instruction.cpp index d24a35e2e945d..97797e99371d6 100644 --- a/llvm/lib/IR/Instruction.cpp +++ b/llvm/lib/IR/Instruction.cpp @@ -317,10 +317,6 @@ void Instruction::setIsExact(bool b) { cast(this)->setIsExact(b); } -void Instruction::setIsDisjoint(bool b) { - cast(this)->setIsDisjoint(b); -} - void Instruction::setNonNeg(bool b) { assert(isa(this) && "Must be zext"); SubclassOptionalData = (SubclassOptionalData & ~PossiblyNonNegInst::NonNeg) | @@ -427,10 +423,6 @@ bool Instruction::isExact() const { return cast(this)->isExact(); } -bool Instruction::isDisjoint() const { - return cast(this)->isDisjoint(); -} - void Instruction::setFast(bool B) { assert(isa(this) && "setting fast-math flag on invalid op"); cast(this)->setFast(B); @@ -544,9 +536,9 @@ void Instruction::copyIRFlags(const Value *V, bool IncludeWrapFlags) { if (isa(this)) setIsExact(PE->isExact()); - if (auto *PD = dyn_cast(V)) - if (isa(this)) - setIsDisjoint(PD->isDisjoint()); + if (auto *SrcPD = dyn_cast(V)) + if (auto *DestPD = dyn_cast(this)) + DestPD->setIsDisjoint(SrcPD->isDisjoint()); // Copy the fast-math flags. if (auto *FP = dyn_cast(V)) @@ -574,9 +566,9 @@ void Instruction::andIRFlags(const Value *V) { if (isa(this)) setIsExact(isExact() && PE->isExact()); - if (auto *PE = dyn_cast(V)) - if (isa(this)) - setIsDisjoint(isDisjoint() && PE->isDisjoint()); + if (auto *SrcPD = dyn_cast(V)) + if (auto *DestPD = dyn_cast(this)) + DestPD->setIsDisjoint(DestPD->isDisjoint() && SrcPD->isDisjoint()); if (auto *FP = dyn_cast(V)) { if (isa(this)) { From 11fa4974aacb6044d7ae47ea6a16f62c72608f7f Mon Sep 17 00:00:00 2001 From: Craig Topper Date: Fri, 24 Nov 2023 08:41:04 -0800 Subject: [PATCH 15/15] fixup! add missing word --- llvm/docs/LangRef.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index ce68540da9a14..e448c5ed5c5d9 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -10013,9 +10013,9 @@ The truth table used for the '``or``' instruction is: | 1 | 1 | 1 | +-----+-----+-----+ -``disjoint`` means for each bit, that bit is zero in at least one of the inputs. -This allows the Or to be treated as an Add since no carry can occur from any -bit. If the disjoint keyword is present, the result value of the ``or`` is a +``disjoint`` means that for each bit, that bit is zero in at least one of the +inputs. This allows the Or to be treated as an Add since no carry can occur from +any bit. If the disjoint keyword is present, the result value of the ``or`` is a :ref:`poison value ` if both inputs have a one in the same bit position. For vectors, only the element containing the bit is poison.