[PATCH] D79792: [AMDGPU] New SIInsertHardClauses pass

[Jay Foad via Phabricator via llvm-commits]

foad marked an inline comment as done.
foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIInsertHardClauses.cpp:116-118
+    BuildMI(*CI.First->getParent(), *CI.First, CI.First->getDebugLoc(),
+            TII->get(AMDGPU::S_CLAUSE))
+        .addImm(CI.Length - 1);
----------------
foad wrote:
> rampitec wrote:
> > arsenm wrote:
> > > Do we need to bundle the claused instructions? What prevents inserting something else between these?
> > Looking at the passes which work after this it does not seem anything will be modified here. But you are probably right, just to be on a safer side.
> I don't know. I haven't noticed any problems in testing. I guess it just relies on running late enough in the pipeline.
Would bundling actually fix the problem? What about other code that modifies bundles, like GCNHazardRecognizer::insertNoopInBundle? It feels like there's an arms race where one side invents mechanisms like bundling to stop instructions being inserted, and the other side goes and inserts instructions in the bundles anyway.

Having said that, I do have some patches in progress to remove GCNHazardRecognizer::insertNoopInBundle. But I don't know if there are any other places where bundles could be modified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79792