-
Notifications
You must be signed in to change notification settings - Fork 15k
[InstCombine] Canonicalize complex boolean expressions into ~((y | z) ^ x) via 3-input truth table #149530
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
base: main
Are you sure you want to change the base?
[InstCombine] Canonicalize complex boolean expressions into ~((y | z) ^ x) via 3-input truth table #149530
Changes from 27 commits
cf2f9db
3a55b19
02807e3
af90743
d066a85
4c86e54
7a2dc67
6772db5
5405486
cb1e164
650a7ab
18e576e
28d4a0f
1fee55f
a39a3b4
23feb15
1a94bba
d19190d
9296d9b
48bd1ca
464d95e
2a905fb
fc2aac4
d557827
5c40046
7bf8caf
abd628d
64fbafd
ecd9669
2b18e01
b238960
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -22,6 +22,7 @@ | |||||
| #include "llvm/IR/PatternMatch.h" | ||||||
| #include "llvm/Transforms/InstCombine/InstCombiner.h" | ||||||
| #include "llvm/Transforms/Utils/Local.h" | ||||||
| #include <bitset> | ||||||
|
|
||||||
| using namespace llvm; | ||||||
| using namespace PatternMatch; | ||||||
|
|
@@ -50,6 +51,230 @@ static Value *getFCmpValue(unsigned Code, Value *LHS, Value *RHS, | |||||
| return Builder.CreateFCmpFMF(NewPred, LHS, RHS, FMF); | ||||||
| } | ||||||
|
|
||||||
| /// This is to create optimal 3-variable boolean logic from truth tables. | ||||||
| /// currently it supports the cases pertaining to the issue 97044. More cases | ||||||
| /// can be added based on real-world justification for specific 3 input cases. | ||||||
| static Value *createLogicFromTable3Var(const std::bitset<8> &Table, Value *Op0, | ||||||
| Value *Op1, Value *Op2, Value *Root, | ||||||
| IRBuilderBase &Builder) { | ||||||
| uint8_t TruthValue = Table.to_ulong(); | ||||||
| auto FoldConstant = [&](bool Val) { | ||||||
| Type *Ty = Op0->getType(); | ||||||
| return Val ? ConstantInt::getTrue(Ty) : ConstantInt::getFalse(Ty); | ||||||
| }; | ||||||
|
|
||||||
| Value *Result = nullptr; | ||||||
| switch (TruthValue) { | ||||||
| default: | ||||||
| return nullptr; | ||||||
| case 0x00: // Always FALSE | ||||||
yafet-a marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| Result = FoldConstant(false); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I don't think we need a helper function for this... (also for true below). |
||||||
| break; | ||||||
| case 0xFF: // Always TRUE | ||||||
| Result = FoldConstant(true); | ||||||
| break; | ||||||
| case 0xE1: // ~((Op1 | Op2) ^ Op0) | ||||||
| { | ||||||
| Value *Or = Builder.CreateOr(Op1, Op2); | ||||||
| Value *Xor = Builder.CreateXor(Or, Op0); | ||||||
| Result = Builder.CreateNot(Xor); | ||||||
| } break; | ||||||
| case 0x60: // Op0 & (Op1 ^ Op2) | ||||||
| { | ||||||
| Value *Xor = Builder.CreateXor(Op1, Op2); | ||||||
| Result = Builder.CreateAnd(Op0, Xor); | ||||||
| } break; | ||||||
| case 0xD2: // ((Op1 | Op2) ^ Op0) ^ Op1 | ||||||
| { | ||||||
| Value *Or = Builder.CreateOr(Op1, Op2); | ||||||
| Value *Xor1 = Builder.CreateXor(Or, Op0); | ||||||
| Result = Builder.CreateXor(Xor1, Op1); | ||||||
| } break; | ||||||
| } | ||||||
|
|
||||||
| return Result; | ||||||
| } | ||||||
|
|
||||||
| static std::tuple<Value *, Value *, Value *> | ||||||
| extractThreeVariablesAndInstructions( | ||||||
yafet-a marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| Value *Root, SmallVectorImpl<Instruction *> &Instructions) { | ||||||
| SmallPtrSet<Value *, 3> Variables; | ||||||
| SmallPtrSet<Value *, 32> Visited; | ||||||
| SmallPtrSet<Value *, 8> RootOperands; | ||||||
| SmallVector<Value *> Worklist; | ||||||
| Worklist.push_back(Root); | ||||||
|
|
||||||
| // Traverse root operands to avoid treating them as leaf variables to prevent | ||||||
| // infinite cycles. | ||||||
| if (auto *RootInst = dyn_cast<Instruction>(Root)) | ||||||
| for (Use &U : RootInst->operands()) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
If you're not using the |
||||||
| RootOperands.insert(U.get()); | ||||||
|
|
||||||
| while (!Worklist.empty()) { | ||||||
| Value *V = Worklist.pop_back_val(); | ||||||
|
|
||||||
| if (!Visited.insert(V).second) | ||||||
| continue; | ||||||
|
|
||||||
| // Due to lack of cost-based heuristic, only traverse if it belongs to this | ||||||
| // expression tree. | ||||||
| bool ShouldTraverse = (V == Root || V->hasOneUse()); | ||||||
|
|
||||||
| if (Value *NotV; match(V, m_Not(m_Value(NotV)))) { | ||||||
| if (auto *I = dyn_cast<Instruction>(V)) | ||||||
| Instructions.push_back(I); | ||||||
| if (ShouldTraverse) | ||||||
| Worklist.push_back(NotV); | ||||||
| continue; | ||||||
| } | ||||||
| if (auto *BO = dyn_cast<BinaryOperator>(V)) { | ||||||
yafet-a marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| if (!BO->isBitwiseLogicOp()) { | ||||||
| if (V == Root) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this happen? Wouldn't this imply root is a non-bitwise op? I think this can be an assert. |
||||||
| return {nullptr, nullptr, nullptr}; | ||||||
| if (!RootOperands.count(V)) | ||||||
| Variables.insert(V); | ||||||
| continue; | ||||||
| } | ||||||
|
|
||||||
| Instructions.push_back(BO); | ||||||
|
|
||||||
| if (ShouldTraverse) { | ||||||
| Worklist.push_back(BO->getOperand(0)); | ||||||
yafet-a marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| Worklist.push_back(BO->getOperand(1)); | ||||||
| } | ||||||
| } else if ((isa<Argument>(V) || isa<Instruction>(V)) && V != Root) { | ||||||
| if (!RootOperands.count(V)) | ||||||
| Variables.insert(V); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if (Variables.size() != 3) | ||||||
| return {nullptr, nullptr, nullptr}; | ||||||
| // Check that all instructions (both variables and computation instructions) | ||||||
| // are in the same BB. | ||||||
| SmallVector<Value *, 3> SortedVars(Variables.begin(), Variables.end()); | ||||||
| BasicBlock *FirstBB = nullptr; | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can simplify this by always comparing to |
||||||
|
|
||||||
| auto CheckSameBB = [&FirstBB](Instruction *I) -> bool { | ||||||
| if (!FirstBB) | ||||||
| FirstBB = I->getParent(); | ||||||
| else if (I->getParent() != FirstBB) | ||||||
| return false; | ||||||
| return true; | ||||||
| }; | ||||||
|
|
||||||
| for (Value *V : SortedVars) | ||||||
| if (auto *I = dyn_cast<Instruction>(V); I && !CheckSameBB(I)) | ||||||
| return {nullptr, nullptr, nullptr}; | ||||||
|
|
||||||
| for (Instruction *I : Instructions) | ||||||
| if (!CheckSameBB(I)) | ||||||
| return {nullptr, nullptr, nullptr}; | ||||||
|
|
||||||
| // Validation that all collected instructions have operands that will be in | ||||||
| // Computed map. | ||||||
| SmallPtrSet<Value *, 32> ValidOperands(Variables.begin(), Variables.end()); | ||||||
| ValidOperands.insert(Instructions.begin(), Instructions.end()); | ||||||
|
|
||||||
| for (Instruction *I : Instructions) { | ||||||
| Value *NotV; | ||||||
| bool IsNot = match(I, m_Not(m_Value(NotV))); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inline |
||||||
|
|
||||||
| if (!IsNot) { | ||||||
| for (Use &U : I->operands()) { | ||||||
| if (!ValidOperands.count(U.get())) | ||||||
| return {nullptr, nullptr, nullptr}; | ||||||
| } | ||||||
| } else if (!ValidOperands.count(NotV)) { | ||||||
| // For NOT: only check the variable operand (constant -1 is handled by | ||||||
| // pattern matcher). | ||||||
| return {nullptr, nullptr, nullptr}; | ||||||
| } | ||||||
| } | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do I understand correctly that what this actually does is discarding cases with constant operands, as everything else should have already been handled in the initial loop? If so, can we bail out on constant operands there already? |
||||||
|
|
||||||
| llvm::sort(SortedVars, [](Value *A, Value *B) { | ||||||
| if (isa<Argument>(A) != isa<Argument>(B)) | ||||||
| return isa<Argument>(A); | ||||||
|
|
||||||
| if (isa<Argument>(A)) | ||||||
| return cast<Argument>(A)->getArgNo() < cast<Argument>(B)->getArgNo(); | ||||||
|
|
||||||
| return cast<Instruction>(A)->comesBefore(cast<Instruction>(B)); | ||||||
| }); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The reason for sorting the instructions is obvious, but I don't really understand why we need/want sorted variables. Is this working around the fact that I think if we do any sorting for the variables it needs to happen at the level of |
||||||
|
|
||||||
| // Sort instructions (Useful until all 256 cases are added). | ||||||
| llvm::sort(Instructions, | ||||||
| [](Instruction *A, Instruction *B) { return A->comesBefore(B); }); | ||||||
|
|
||||||
| return {SortedVars[0], SortedVars[1], SortedVars[2]}; | ||||||
| } | ||||||
|
|
||||||
| /// Evaluate a boolean expression with bit-vector inputs for all 8 combinations. | ||||||
| static std::optional<std::bitset<8>> | ||||||
| evaluateBooleanExpression(Value *Expr, Value *Op0, Value *Op1, Value *Op2, | ||||||
| const SmallVector<Instruction *> &Instructions) { | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| // Initialize bit-vector values for the 3 variables as: | ||||||
| // Op0: 0b11110000 (true for combinations 000,001,010,011) | ||||||
| // Op1: 0b11001100 (true for combinations 000,001,100,101) | ||||||
| // Op2: 0b10101010 (true for combinations 000,010,100,110) | ||||||
| SmallDenseMap<Value *, std::bitset<8>> Computed; | ||||||
| Computed[Op0] = std::bitset<8>(0xF0); // 11110000 | ||||||
| Computed[Op1] = std::bitset<8>(0xCC); // 11001100 | ||||||
| Computed[Op2] = std::bitset<8>(0xAA); // 10101010 | ||||||
|
|
||||||
| for (Instruction *I : Instructions) { | ||||||
| Value *NotV; | ||||||
| if (match(I, m_Not(m_Value(NotV)))) { | ||||||
| Computed[I] = ~Computed.at(NotV); // Bitwise NOT | ||||||
| } else if (auto *BO = dyn_cast<BinaryOperator>(I)) { | ||||||
| auto &LHS = Computed.at(BO->getOperand(0)); | ||||||
| auto &RHS = Computed.at(BO->getOperand(1)); | ||||||
|
|
||||||
| switch (BO->getOpcode()) { | ||||||
| case Instruction::And: | ||||||
| Computed[I] = LHS & RHS; // Bitwise AND | ||||||
| break; | ||||||
| case Instruction::Or: | ||||||
| Computed[I] = LHS | RHS; // Bitwise OR | ||||||
| break; | ||||||
| case Instruction::Xor: | ||||||
| Computed[I] = LHS ^ RHS; // Bitwise XOR | ||||||
| break; | ||||||
| default: | ||||||
| llvm_unreachable("Unexpected opcode in boolean expression evaluation"); | ||||||
| } | ||||||
| } | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about the case where it's neither Not nor BinaryOperator? I assume it can't happen, in which case we should make that dyn_cast above a cast. |
||||||
| } | ||||||
|
|
||||||
| return std::bitset<8>(Computed.at(Expr)); | ||||||
| } | ||||||
|
|
||||||
| // Entry point for the 3-variable boolean expression folding. Handles early | ||||||
| // returns. | ||||||
| static Value *foldThreeVarBoolExpr(Instruction &Root, | ||||||
yafet-a marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| InstCombiner::BuilderTy &Builder) { | ||||||
|
|
||||||
| auto &BO = cast<BinaryOperator>(Root); | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Directly accept BinaryOperator as the argument? (It will downcast to Instruction where needed.) |
||||||
| assert(BO.isBitwiseLogicOp() && "Unexpected opcode for boolean expression"); | ||||||
|
|
||||||
| if (!isa<BinaryOperator>(BO.getOperand(0)) || | ||||||
| !isa<BinaryOperator>(BO.getOperand(1))) | ||||||
| return nullptr; | ||||||
|
|
||||||
| SmallVector<Instruction *> Instructions; | ||||||
| auto [Op0, Op1, Op2] = | ||||||
| extractThreeVariablesAndInstructions(&Root, Instructions); | ||||||
| if (!Op0 || !Op1 || !Op2) | ||||||
| return nullptr; | ||||||
|
|
||||||
| auto Table = evaluateBooleanExpression(&Root, Op0, Op1, Op2, Instructions); | ||||||
| if (!Table) | ||||||
| return nullptr; | ||||||
|
|
||||||
| return createLogicFromTable3Var(*Table, Op0, Op1, Op2, &Root, Builder); | ||||||
| } | ||||||
|
|
||||||
| /// Emit a computation of: (V >= Lo && V < Hi) if Inside is true, otherwise | ||||||
| /// (V < Lo || V >= Hi). This method expects that Lo < Hi. IsSigned indicates | ||||||
| /// whether to treat V, Lo, and Hi as signed or not. | ||||||
|
|
@@ -2400,6 +2625,9 @@ Instruction *InstCombinerImpl::visitAnd(BinaryOperator &I) { | |||||
| if (Instruction *Phi = foldBinopWithPhiOperands(I)) | ||||||
| return Phi; | ||||||
|
|
||||||
| if (Value *Canonical = foldThreeVarBoolExpr(I, Builder)) | ||||||
| return replaceInstUsesWith(I, Canonical); | ||||||
|
|
||||||
| // See if we can simplify any instructions used by the instruction whose sole | ||||||
| // purpose is to compute bits we don't care about. | ||||||
| if (SimplifyDemandedInstructionBits(I)) | ||||||
|
|
@@ -3905,6 +4133,9 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) { | |||||
| if (Instruction *Phi = foldBinopWithPhiOperands(I)) | ||||||
| return Phi; | ||||||
|
|
||||||
| if (Value *Canonical = foldThreeVarBoolExpr(I, Builder)) | ||||||
| return replaceInstUsesWith(I, Canonical); | ||||||
|
|
||||||
| // See if we can simplify any instructions used by the instruction whose sole | ||||||
| // purpose is to compute bits we don't care about. | ||||||
| if (SimplifyDemandedInstructionBits(I)) | ||||||
|
|
@@ -5055,6 +5286,9 @@ Instruction *InstCombinerImpl::visitXor(BinaryOperator &I) { | |||||
| if (Instruction *Phi = foldBinopWithPhiOperands(I)) | ||||||
| return Phi; | ||||||
|
|
||||||
| if (Value *Canonical = foldThreeVarBoolExpr(I, Builder)) | ||||||
| return replaceInstUsesWith(I, Canonical); | ||||||
|
|
||||||
| if (Instruction *NewXor = foldXorToXor(I, Builder)) | ||||||
| return NewXor; | ||||||
|
|
||||||
|
|
||||||
Uh oh!
There was an error while loading. Please reload this page.