[PATCH] D133723: [AMDGPU][GFX11] Use VGPR_32_F128 for VOP1,2,C

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp:126
   }
+  if (llvm::AMDGPU::isTrue16Inst(Op))
+    return false;
----------------
Don't need the `llvm::` (there are a few occurrences in this file and in SIShrinkInstructions.cpp).


================
Comment at: llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp:127
+  if (llvm::AMDGPU::isTrue16Inst(Op))
+    return false;
   if (const auto *SDst = TII->getNamedOperand(MI, AMDGPU::OpName::sdst)) {
----------------
Maybe add a brief comment here? These instructions are "shrinkable" but we don't want to shrink them pre-RA because <reasons>.


================
Comment at: llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp:606-608
+    assert(
+        (TII->get(OrigOp).Size != 4 || !llvm::AMDGPU::isTrue16Inst(OrigOp)) &&
+        "There should not be e32 True16 instructions pre-RA");
----------------
This may be true but I don't see why you're asserting it //here// in particular. Couldn't this go into isShrinkable, near the other check that you added?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133723