[llvm] [InstCombine] Do not use operand info in `replaceInInstruction` (PR #99492)

via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 18 08:32:15 PDT 2024


================
@@ -6796,12 +6797,15 @@ bool llvm::isSafeToSpeculativelyExecuteWithOpcode(
   case Instruction::URem: {
     // x / y is undefined if y == 0.
     const APInt *V;
-    if (match(Inst->getOperand(1), m_APInt(V)))
+    if (UseOperandInfo && match(Inst->getOperand(1), m_APInt(V)))
----------------
goldsteinn wrote:

Agreed, I just checked out my old patch, what I had done was:

I had added the nebulus bool `PreservesOpCharacteristics` which if `true` promised to not change the operands in a way the violated any of the analysis done in `isSafeToSpeculativeExecute` (although there was no real means of coordinating the analysis being done and the transform, I just eyeballed the ~50 instances I found in the code).

I also had
```
+/// Similiar to the above, but provide specific operands to be used in `I`.
+/// NewOperands.size() must equal I.getNumOperands().
+bool isSafeToExecuteWithTheseOperands(const Instruction *I,
+                                      const ArrayRef<Use> NewOperands,
+                                      const Instruction *CtxI = nullptr,
+                                      AssumptionCache *AC = nullptr,
+                                      const DominatorTree *DT = nullptr,
+                                      const TargetLibraryInfo *TLI = nullptr);
```
but this was a bit naive, as constructing/destructing non-const operands is a pain.

Without creating unique helpers for the places that want to replace operands, I'm not really sure a way around either asking what operands or some nebulus contract that what they are doing doesn't violate the analysis.

https://github.com/llvm/llvm-project/pull/99492


More information about the llvm-commits mailing list