[PATCH] D109052: [AMDGPU][GlobalISel] Fix waterfall loops

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp:3142-3146
+    while (Start->getOpcode() != FrameSetupOpcode)
+      --Start;
+    MachineBasicBlock::iterator End(&MI);
+    while (End->getOpcode() != FrameDestroyOpcode)
+      ++End;
----------------
sebastian-ne wrote:
> arsenm wrote:
> > I'm not sure you can guarantee this is the range that needs to be moved. Could other instructions have been moved across these between the IRTranslator and here?
> The running passes seem to be
> - IRTranslator
> - AMDGPUPreLegalizerCombiner
> - Localizer
> - Legalizer
> - AMDGPUPostLegalizerCombiner
> - RegBankSelect
> 
> It’s fine if VALU instructions are moved inside the loop, but SALU instructions would be a problem.
> 
> Can we prevent instructions from being moved into the call code?
I was thinking of doing this before by using virtual registers for the call pseudo and expanding to physical registers later, but it seems like overkill. Practically speaking we don't really have code reordering right now that would present an issue, and don't see why we would ever add it. The localizer does some level of reordering, but it's really just a simple sink to uses. In practice we'll only see copies inside this range. Overall I can't see a real issue doing it this way, but it still feels wrong since there's no formal guarantee


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109052/new/

https://reviews.llvm.org/D109052