[PATCH] D51726: [AMDGPU] Remove non-instructions from GCNHazardRecognizer buffer

[Carl Ritson via Phabricator via llvm-commits]

critson marked 2 inline comments as done.
critson added inline comments.


================
Comment at: lib/Target/AMDGPU/GCNHazardRecognizer.cpp:263-265
       unsigned Opcode = MI->getOpcode();
-      if (Opcode == AMDGPU::DBG_VALUE || Opcode == AMDGPU::IMPLICIT_DEF ||
-          Opcode == AMDGPU::INLINEASM)
+      if (Opcode == AMDGPU::INLINEASM)
         continue;
----------------
nhaehnle wrote:
> nhaehnle wrote:
> > Please remove INLINEASM here, as well. A test case would be good as well.
> > 
> > There is no guarantee that inline assembly contains actual instructions, and in fact there are cases where Mesa currently generates fake inline assembly as a workaround.
> Actually, scratch that, I got my wires crossed about the logic of this code. This part of it should be fine?
> 
> However, I think the point stands that the same issue that affects DBG_VALUEs with respect to the buffer can also affect inline assembly instructions.
We cannot remove INLINEASM from the buffer as INLINEASM can interact with registers and cause the introduction of NOPs.
There is a test for this behaviour in hazard-inlineasm.mir.
If a large number of INLINEASM are overflowing the buffer then this will require a separate workaround (or increased buffer size).


https://reviews.llvm.org/D51726