[PATCH] D119475: [AMDGPU] Add scheduler pass to rematerialize trivial defs

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp:933
+    Register Reg = Def->getOperand(0).getReg();
+    TII->reMaterialize(*InsertPos->getParent(), InsertPos, Reg,
+                       Def->getOperand(0).getSubReg(), *Def, *TRI);
----------------
rampitec wrote:
> Actually you cannot just rematerialize it without checking all uses are available. Check LiveRangeEdit::allUsesAvailableAt and LiveRangeEdit::canRematerializeAt, this is the checks you need to perform before adding an instruction to the list.
You could since you are refusing instructions with any uses.


================
Comment at: llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp:966
+
+  for (const MachineOperand &MO : MI.operands())
+    if (MO.isReg() && MO.isUse() && MO.getReg().isVirtual())
----------------
rampitec wrote:
> Use TII->isReallyTriviallyReMaterializable() instead.
Actually I see that you are refusing any uses, so you do not need to do the same checks as LRE before rematerialization. That's fine, but needs a comment. Disregard a comment about isReallyTriviallyReMaterializable, isTriviallyReMaterializable calls it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119475