[PATCH] D119696: [AMDGPU] Improve v_cmpx usage on GFX10.3.

[Sebastian Neubauer via Phabricator via llvm-commits]

sebastian-ne added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp:356
+      TII->getNamedOperand(SaveExec, AMDGPU::OpName::src0);
+  if (!SaveExecSrc0->isReg() || SaveExecSrc0->getSubReg())
+    return nullptr;
----------------
tsymalla wrote:
> sebastian-ne wrote:
> > tsymalla wrote:
> > > sebastian-ne wrote:
> > > > There’s quite a lot of these isReg and !getSubReg checks in this patch. What are they guarding? I replaced them by asserts and there was no amdgpu codegen test that failed. (Leaving aside the isReg test for v_cmp arguments, which can also be constants.)
> > > Regarding the v_cmp arguments, these are constants as you mentioned so these checks make sense.
> > > However, I think we can safely assume that the check for s_and_saveexec will always pass as the arguments are registers, so I'm going to remove these.
> > Did you remove any of these checks? I still see a lot of isReg and getSubReg checks
> Yes, I removed the check around the s_and_saveexec operand. For the v_cmp checks, either src0 or src1 could be a register and shouldn't be a subregister, so I think these checks still make sense.
Ah, I missed that. I’m still wondering what the getSubReg checks do?
There’s no change on the llvm tests with or without them. I guess a test that hits these code paths would be nice?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119696