[PATCH] D117562: [AMDGPU] Sink immediate VGPR defs if high RP

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec requested changes to this revision.
rampitec added a comment.
This revision now requires changes to proceed.

The code as written does not guarantee it will increase the occupancy if a def was sunk. Moreover it will sink defs even in regions with low RP. Essentially it may easily pessimise the code without improving anything. You really need to check that sinking is beneficial.

It is also irrelevant to check for occupancy 1. If you are doing that your main criteria is if sinking will increase the occupancy or not as long as current occupancy is not at requested maximum.

Needs tests, plenty of tests.



================
Comment at: llvm/lib/Target/AMDGPU/GCNPreRAOptimizations.cpp:97
+  MachineInstr *VGPRImmUse = nullptr;
+  std::map<unsigned, unsigned> SeenSubRegDefs;
+
----------------
No need to use sd::map, you can use a map from llvm.


================
Comment at: llvm/lib/Target/AMDGPU/GCNPreRAOptimizations.cpp:112
+      MachineOperand &Op = I.getOperand(0);
+      if (!Op.isReg())
+        break;
----------------
Is this ever possible to have a def w/o a def?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117562