- Once an instruction is simplified, foldable candidates from it should be invalidated or skipped as the operand index is no longer valid.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Time | Test | |
---|---|---|
300 ms | x64 windows > LLVM.CodeGen/XCore::threads.ll |
Event Timeline
llvm/lib/Target/AMDGPU/SIFoldOperands.cpp | ||
---|---|---|
1260 | Doesn't this add a second mechanism to avoid the same problem? We already check isUseMIInFoldList to avoid revisiting |
llvm/lib/Target/AMDGPU/SIFoldOperands.cpp | ||
---|---|---|
1260 | That one is used to prevent adding a commuted instr into the candidate list. But the case here is that both operands could be folded. Also, if used here, that check is too expensive? That candidate list will be scanned in square times. |
llvm/lib/Target/AMDGPU/SIFoldOperands.cpp | ||
---|---|---|
1260 | Should we just use a SetVector for FoldList then? |
llvm/lib/Target/AMDGPU/SIFoldOperands.cpp | ||
---|---|---|
1260 | That list is a list of operand foldable, i.e., the pair MI and one of its operand being folded. If we change that to be keyed by MI, besides major data structure change, we may add extra overhead (set vs list) when building that candidate list. Considering that tryFoldInst only simplifies V_CNDMASK so far, is that overhead too big to justify the skip list here? |
llvm/lib/Target/AMDGPU/SIFoldOperands.cpp | ||
---|---|---|
1260 | Hmmm. In principle tryFoldInst would erase / replace the instruction, but it just happens to not. I do think the way this pass works is backwards, but I guess this is fine for now |
llvm/lib/Target/AMDGPU/SIFoldOperands.cpp | ||
---|---|---|
1260 | Thanks for the code review |
Doesn't this add a second mechanism to avoid the same problem? We already check isUseMIInFoldList to avoid revisiting