PHI Elimination pass relies on TargetInstrInfo::isBlockPrologue/isTerminator target hooks to determine the COPY insertion points.
In some cases this is not sufficient. Divergent control flow requires instructions that belong to block prologue and consume registers defined by the PHI node in the same block.
2 new hooks were added to allow target to override the default COPY placement if necessary or fall through to the base class method otherwise.
Details
Diff Detail
Event Timeline
llvm/lib/CodeGen/PHIElimination.cpp | ||
---|---|---|
280 | return inserted instruction and assign iterator to it to AfterPHIsIt | |
410 | returned InsertPos and NewSrcInstr is the same, use only returned value, remove reference arg from the interface. | |
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp | ||
6398 | use the same name for LastPHIIt arg as in the declaration | |
6400 | auto | |
6407 | search first user from the beginning of the block? | |
llvm/lib/Target/AMDGPU/SIInstrInfo.h | ||
957 | void -> MachineInstr*, remove reference to InsPt | |
963 | remove reference from InsPt |
Would be good to have test coverage either in the same patch or have dependent patch explicitly marked as such.
llvm/include/llvm/CodeGen/TargetInstrInfo.h | ||
---|---|---|
1647 | Should use Register instead of unsigned |
The patch really changes AMDGPU PHI elimination and leaves other targets behavior unaffected.
So the test that I will add make sense for AMDGPU only.
Is it okay?
@alex-t This is failing on EXPENSIVE_CHECKS builds: http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/19594/steps/test-check-all/logs/stdio
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp | ||
---|---|---|
6417 | So you think it is always scalar and always 64 bit? |
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp | ||
---|---|---|
6417 | Since only case is SI_CF yes. |
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp | ||
---|---|---|
6417 | Oh... yes. in WAVE32 it will be S_MOV_B32_term |
llvm/lib/Target/AMDGPU/SIInstrInfo.cpp | ||
---|---|---|
6417 | What ensures that is just SI_CF? Why it cannot be some another PHI? |
Delete to