[PATCH] D134151: [AMDGPU][GFX11] Mitigate VALU mask write hazard

Carl Ritson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 20 19:13:27 PDT 2022


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


================
Comment at: llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp:2719
+
+  const MachineOperand *SDSTOp = TII.getNamedOperand(*MI, AMDGPU::OpName::sdst);
+  if (!SDSTOp || !SDSTOp->isReg())
----------------
rampitec wrote:
> You seem to start from the instruction in the middle and have no check for the 3rd instruction. This makes the w/a overrestrictive as far as I understand. This maybe fine, but probably deserves a comment.
Yes, this intentionally does not do a forward check to look for hazard expiry.
>From my analysis we rarely reach expiry, <10% of the time in graphics.
So I considered the forward check a waste of compile time.
I will add a comment to explain this.


================
Comment at: llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp:2725
+  if (HazardReg == AMDGPU::EXEC ||
+      HazardReg == AMDGPU::EXEC_LO ||
+      HazardReg == AMDGPU::EXEC_HI ||
----------------
rampitec wrote:
> A mask can only be a 64 bit register in wave64. You could check the size and bail rather then listing 32 bit registers.
Mask can only be 64 bit, but SALU write to either 32 bit half is a hazard.


================
Comment at: llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp:2732
+    switch (I.getOpcode()) {
+    case AMDGPU::V_ADDC_U32_e32:
+    case AMDGPU::V_CNDMASK_B16_e32:
----------------
rampitec wrote:
> Isn't that less error prone to just check for an implicit vcc use? I assume it shall not be true unless it reads a mask.
I am not sure I understand?
These instructions always implicitly read VCC.


================
Comment at: llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp:2741
+      return HazardReg == AMDGPU::VCC ||
+             HazardReg == AMDGPU::VCC_LO ||
+             HazardReg == AMDGPU::VCC_HI;
----------------
rampitec wrote:
> As far as I understand this can only be a VCC too.
As above, my understanding is partial overlap is also a hazard.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134151



More information about the llvm-commits mailing list