[PATCH] D46756: [AMDGPU] Reworked SIFixWWMLiveness

Fri Jun 15 05:50:27 PDT 2018

nhaehnle added a comment.

I've had some time to let this sink in now.

Let me start by saying that I'm very skeptical of any approach that relies on LoopInfo or RegionInfo for correctness, and case distinctions in general.

That said, I do like the idea of getting rid of implicit uses on WWM, and instead turning defs of variables into partial defs. That fundamentally makes sense.

I also think your second FIXME is spot-on: if the register allocator does any kind of splitting / spilling inside WWM code, things are likely to get screwed up. I couldn't think of a way to fix that without going into the guts of the register allocator, so the following just ignores that problem entirely for now.

How about the following alternative logic for where partial defs are needed. Move this pass to before PHI elimination (but still after WQM), and consider all PHIs of vector registers. For every operand X of the PHI node consider its unique def D. If any of the defs of the //other// operands of the PHI node can reach[1] D via a WWM region, then add an appropriate implicit use to D. In the general case, this may require creating new PHI nodes to preserve SSA form, so perhaps the MachineSSAUpdater can be used for some of this (this would also help with adding IMPLICIT_DEFs where needed).

With this approach, we should be able to feel much more confident about the correctness of it all (except for the issue with register spills), and having fewer steps between PHI elimination and register allocation is always a good thing. It also preserves the nice property of your approach that we don't add excessive implicit uses.

If a normal graph walk is used to determine reachability in [1], then this is a somewhat conservative approximation in some uniform control flow cases, but I hope we can accept this as a first step. I haven't given much thought yet to how we could do better in general.

Repository:
  rL LLVM

https://reviews.llvm.org/D46756