Users, especially the Attributor, might replace multiple operands at
once. The actual implementation of simplifyWithOpReplaced is able to
handle that just fine, the interface was simply not allowing to replace
more than one operand at a time. This is exposing a more generic
interface without intended changes for existing code.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/include/llvm/Analysis/InstructionSimplify.h | ||
---|---|---|
328–333 |
llvm/include/llvm/Analysis/InstructionSimplify.h | ||
---|---|---|
332 | Should this be ArrayRef? |
I don't think simplifyWithOpReplaced() is the right starting point for this. This function has a specific purpose and it has complex and dangerous semantics.
I think what you really want is something along the lines of ConstantFoldInstruction vs ConstantFoldInstOperands. That is, you should take SimplifyInstruction and rewrite it into a SimplifyInstOperands function that takes the operands as an explicit ArrayRef, and then call that from SimplifyInstruction. That will ensure that you can actually simplify all instructions that InstSimplify supports, rather than just the peculiar subset that simplifyWithOpReplaced deals with.
Change approach to use the SimplifyInstruction interface instead. Tests not updated yet.
@nikic Like this?
I needed to use NewOps for load simplification to make it as powerful as the former approach. Also did the same to PHI simplification because it seemed easy.
Not sure if it makes sense to commit some parts of this separately, maybe not.
llvm/lib/Analysis/InstructionSimplify.cpp | ||
---|---|---|
6221–6222 | assert(NewOps.size() == I.getNumOperands() && ""); |
clang-tidy: warning: invalid case style for function 'SimplifyFNegInst' [readability-identifier-naming]
not useful