[llvm] [AMDGPU] Fix LSR cost comparator that regressed GFX9+ occupancy (PR #204344)

[Yuyang Zhang via llvm-commits]

================
@@ -1858,21 +1858,20 @@ InstructionCost GCNTTIImpl::getScalingFactorCost(Type *Ty, GlobalValue *BaseGV,
 
 bool GCNTTIImpl::isLSRCostLess(const TTI::LSRCost &A,
                                const TTI::LSRCost &B) const {
-  // Favor lower per-iteration work over preheader/setup costs.
-  // AMDGPU lacks rich addressing modes, so ScaleCost is folded into the
-  // effective instruction count (base+scale*index requires a separate ADD).
+  // GFX9+ occupancy is VGPR-bound, so register pressure must dominate the LSR
+  // cost: NumRegs/AddRecCost outrank EffInsns (Insns + ScaleCost), so LSR does
+  // not add registers to save a few per-iteration instructions.
   unsigned EffInsnsA = A.Insns + A.ScaleCost;
   unsigned EffInsnsB = B.Insns + B.ScaleCost;
-
-  return std::tie(EffInsnsA, A.NumIVMuls, A.AddRecCost, A.NumBaseAdds,
-                  A.SetupCost, A.ImmCost, A.NumRegs) <
-         std::tie(EffInsnsB, B.NumIVMuls, B.AddRecCost, B.NumBaseAdds,
-                  B.SetupCost, B.ImmCost, B.NumRegs);
+  return std::tie(A.NumRegs, A.AddRecCost, EffInsnsA, A.NumIVMuls,
+                  A.NumBaseAdds, A.ImmCost, A.SetupCost) <
+         std::tie(B.NumRegs, B.AddRecCost, EffInsnsB, B.NumIVMuls,
+                  B.NumBaseAdds, B.ImmCost, B.SetupCost);
 }
 
 bool GCNTTIImpl::isNumRegsMajorCostOfLSR() const {
-  // isLSRCostLess de-prioritizes register count; keep consistent.
-  return false;
+  // NumRegs leads isLSRCostLess, so register count is the major cost.
----------------
yuyzhang512 wrote:

Thanks, I agree the comment is confusing. My intent was just to keep the existing hook consistent with the updated cost ordering. I’ll drop the comment and keep the change focused.

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