[PATCH] D128252: [AMDGPU] VGPR to SGPR copies lowering

[Alexander via Phabricator via llvm-commits]

alex-t added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIFixSGPRCopies.cpp:575
+  // Is kept aside to process V2S copies before the rest of the stuff
+  lowerVGPR2SGPRCopies(MF);
+
----------------
arsenm wrote:
> What's the advantage of splitting this into a separate loop?
We, basically, have 3 phases - analysis that collect information related to the particular copy, a decision that is made based on the information collected, and lowering that lowers the copy according to the decision made.
If processed into the main loop now, the info collected is changed by the REG_SEQUENCE & Co lowering and appears not actual at the moment when copies are lowered. The simplest example is the copy that has a SALU chain long enough to be v_readfirstlane_b32 but the nearest user is REG_SEQUENCE with one VGPR input.
We collect information that suggests we convert the copy into the v_readfirstlane_b32 but REG_SEQUENCE will be passed to moveToVALU because of the VGPR operand.
The only advantage is the possibility to split the change into one, analyzing and lowering just the copies, and another one, extending the algorithm to process the REG_SEQUENCE, PHIs & Co.
The latter is the next change and is upcoming soon.
The plan is to incrementally integrate both and then optimize compile time collecting the information in one loop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128252