[PATCH] D87585: [AMDGPU] Dynamically clear renamable to avoid constant bus errors

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIFixRenamableFlags.cpp:96
+          Register Reg = Use.getReg();
+          if (RI.isVGPR(*MRI, Reg) || RI.isAGPR(*MRI, Reg)) {
+            if (llvm::all_of(VGPRs, [Reg](unsigned VGPR) {
----------------
Same as RI.isVectorRegister().


================
Comment at: llvm/lib/Target/AMDGPU/SIFixRenamableFlags.cpp:98
+            if (llvm::all_of(VGPRs, [Reg](unsigned VGPR) {
+                  return VGPR != Reg;
+                })) {
----------------
Looks like you can get the same result with a Set/SetVector.


================
Comment at: llvm/lib/Target/AMDGPU/SIFixRenamableFlags.cpp:130
+          Register Reg = Use.getReg();
+          if (RI.isVGPR(*MRI, Reg) || RI.isAGPR(*MRI, Reg)) {
+            Use.setIsRenamable(false);
----------------
RI.isVectorRegister().


================
Comment at: llvm/lib/Target/AMDGPU/SIInstrInfo.cpp:3599
+        if (llvm::all_of(SGPRsUsed, [this, SGPRUsed](unsigned SGPR) {
+              return !RI.regsOverlap(SGPRUsed, SGPR);
+            })) {
----------------
This does not seem to be correct? I.e. s[0:1] and s1 overlap, but count as two constant bus operands. I think even s[0:1] and s0 are separate operands. I see that it is copied from below, but seems to be an error anyway?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87585