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

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec accepted this revision.
rampitec added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp:2719
+
+  const MachineOperand *SDSTOp = TII.getNamedOperand(*MI, AMDGPU::OpName::sdst);
+  if (!SDSTOp || !SDSTOp->isReg())
----------------
critson wrote:
> 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.
Thanks for adding the comment. In fact forward search would not work, we do not have such mechanism. There are cases of 3 instruction hazards where we always start from the last instruction, but that would be an overkill here.


================
Comment at: llvm/lib/Target/AMDGPU/GCNHazardRecognizer.cpp:2732
+    switch (I.getOpcode()) {
+    case AMDGPU::V_ADDC_U32_e32:
+    case AMDGPU::V_CNDMASK_B16_e32:
----------------
critson wrote:
> 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.
I mean instead of listing instruction opcodes it can use a general check is there is an implicit $vcc operand. But OK, if you think this list is comprehensive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134151