[PATCH] D128681: [AMDGPU][ScheduleDAG] Check for CopyToReg PhysReg clobbers in pre-RA-sched

[Jeffrey Byrnes via Phabricator via llvm-commits]

jrbyrnes added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp:1311
+    // Allow multiple uses of same def
+    if (Node && LiveRegDefs[*AliasI]->getNode() == Node)
+      continue;
----------------
rampitec wrote:
> Do you really want to insert it if Node is nullptr?
Hey Stas -- thanks for comments.

The are two theoretical ways `Node` will be `nullptr`. 1. We are using default value, 2. The `SDNode` we passed is actually a `nullptr`. Neither way creates a problem for this condition.

1. All invocations of `CheckForLiveRegDef` prior to this patch will use the default value. Since this condition is new, in order to maintain the prior behavior, we ignore this condition for the default value.

2. I assume that this is not possible, and I believe this is a valid assumption. The `SDNode` we pass is the `Node` that defines the Src `SDValue` in `CopyToReg`. I think the Src for `CopyToReg` must be coming from `SDNode` that is not `nullptr`. I can express this assumption via assert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128681