[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.


http://reviews.llvm.org/D18162





More information about the llvm-commits mailing list