[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