[PATCH] D144099: [AMDGPU] Fold more AGPR copies/PHIs in SIFoldOperands

[Pierre van Houtryve via Phabricator via llvm-commits]

Pierre-vh marked 2 inline comments as done.
Pierre-vh added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIFoldOperands.cpp:1697
+          getRegOpRC(*MRI, *TRI, Copy->getOperand(1));
+      if (ARC && ARC != CopyInRC)
+        return false;
----------------
arsenm wrote:
> Direct class equality checks are usually the wrong thing to do. Something like isSubclassEq or constrain to compatible subclass. Don't think there's any practical difference in this case
Here I want to make sure the RC is identical because I can create new register/copies to that RC. Should I still replace equality checks with isSubclassEq?


================
Comment at: llvm/lib/Target/AMDGPU/SIFoldOperands.cpp:1847
+  // Look at all AGPR Phis and collect the register + subregister used.
+  DenseMap<std::pair<Register, unsigned>, std::vector<MachineOperand *>>
+      RegToMO;
----------------
arsenm wrote:
> Don't see why you need to build this map/vector. You can just start inserting the instructions after all the phis as you process each one
I only insert VGPR temps if there is more than one use across all PHIs, so I first collect all the PHIs then check the operands I collected. A worklist seems more appropriate, though I could maybe do it with users in one go without building the map but it seems like it would be more complicated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144099