[llvm] [MachineLateInstrsCleanup] Reuse redundant spill-slot reloads (PR #220180)

[Jonas Paulsson via llvm-commits]

JonPsson1 wrote:

I agree that it seems way too complicated to do this during regalloc - if these reloads are in high pressure regions it would probably not be trivial at all to achive optimized allocations according to your previous comments abut the stages. This is kind of the same reasoning as with the pass as it is now - it handles things "too late" because it would be overly complex to do them optimally early.

As mentioned earlier, I do see some cases of improvements looking at the code, which are nice, but at least on SystemZ the performance isn't really affected. So given that, and that the patch isn't really quite trivial, I probably would leave it out just in order to keep it simple. Do you see any performance improvement on your side that has real value?

I wish there was a simpler way that would be acceptable and work without adding a lot of extra concerns about memory. There are several details I might hope are over-conservative and if we can decide to not worry about them, the patch would be simpler:

- Can a call really overwrite the spill-slots of the caller?
- Can an instruction with unmodelled side-effects affect spill slots?
- Would we have to worry about a store without memory operands affecting spills lots? That seems unnecessary in the bigger picture.

I wonder if we could get away from all this conservatism to the point where we have spill slots that are only that, and then let the target trivially decide if a store to a spill slot would affect the same slot, like:

```
lg      %r0, 456(%r15)   // SP + 456
stg     %r1, 464(%r15)  // SP + 464: This one is trivially non-aliasing as it has a different offset
lg      %r0, 456(%r15)   // SP + 456
```

If we added a new hook that asked the target about this in this particular context, it could - after FI elimination - feel safe if it knew exactly how it accesses all spill slots that a spill slot is in fact never touched unless done by its emitted spill instruction for it.

Iff the above is indeed safe and not possible to argue against given any comment or documentation (there may well be), it seems like a reasonable extension to this pass to me. We would only need to add a little extra check to detect the spill instructions we need to know about. In fact, this should be even more effective with more redundant reloads cleaned away.

Does this make sense, or am I forgetting something here?


https://github.com/llvm/llvm-project/pull/220180