[PATCH] D95748: AMDGPU: Fix dbg_value handling when forming soft clause bundles
Matt Arsenault via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 1 14:31:32 PST 2021
arsenm added inline comments.
================
Comment at: llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp:388
RPT.reset(*B, &LiveRegsCopy);
+ DbgInstrs.clear();
+
----------------
rampitec wrote:
> dfukalov wrote:
> > Why don't just create DbgInstrs here?
> Right, it can be local to this block and cleared/destroyed after re-insertion. It will take less memory.
This avoids reallocating on subsequent bundles if it's necessary
================
Comment at: llvm/lib/Target/AMDGPU/SIFormMemoryClauses.cpp:390-392
+ auto BundleNext = I;
+ for (auto BI = I; BI != Next; BI = BundleNext) {
+ BundleNext = std::next(BI);
----------------
scott.linder wrote:
> When does `Next != BundleNext` after this `for` terminates? It seems like you should be able to just use `Next` below when re-inserting the DBG instructions?
It doesn't make a difference
================
Comment at: llvm/test/CodeGen/AMDGPU/soft-clause-dbg-value.mir:2
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx906 -mattr=+xnack -run-pass=si-form-memory-clauses -verify-machineinstrs -o - %s | FileCheck %s
+
----------------
dfukalov wrote:
> Am I right that we need `phi-node-elimination,` before to remove `IsSSA` property to avoid verifier' fail?
> Or, dummy copy of first instruction `%0:sreg_64 = COPY $sgpr4_sgpr5 ; defeat IsSSA detection` can be added.
No. The pass cleared properties now clears SSA
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95748/new/
https://reviews.llvm.org/D95748
More information about the llvm-commits
mailing list