[PATCH] D18162: AMDGPU: Add SIWholeQuadMode pass
Nicolai Hähnle via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 17 08:29:20 PDT 2016
nhaehnle added a comment.
In http://reviews.llvm.org/D18162#376804, @tstellarAMD wrote:
> The target independent changes might get more visibility if they were in a separate patch.
> > The changes in this patch expose a problem with the second machine
> > scheduling pass: target independent instructions like COPY implicitly
> > use EXEC when they operate on VGPRs, but this fact is not encoded in
> > the MIR. This can lead to miscompilation because instructions are
> > moved past changes to EXEC.
> Is the Scheduler the only pass we need to worry about? Would we be able to avoid the problem by implementing TargetInstrInfo::isSchedulingBoundry() ?
Hmm, I wasn't aware of that function. Yes, customizing that function so that it considers instructions with EXEC defs as scheduling boundaries should work as well.
It's a slightly more conservative solution because it also prevents movement of SALU and SMEM instructions. Then again, it should not impact the initial DAG-based scheduling. Also, I notice that the target-independent code mentions that considering stack pointer modifications to be scheduling boundaries is profitable, and the same logic likely applies to EXEC. Plus, it's a more robust solution.
I'll modify the patch to use isSchedulingBoundary instead.
More information about the llvm-commits