[PATCH] D122489: [AMDGPU] Fix adding modifiers when creating v_cmpx instructions.

[Thomas Symalla via Phabricator via llvm-commits]

tsymalla added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/SIOptimizeExecMasking.cpp:454
 
-  if (AMDGPU::getNamedOperandIdx(NewOpcode, AMDGPU::OpName::src0_modifiers) !=
-      -1)
-    Builder.addImm(0);
+  auto AddModifierOrZero = [&](unsigned OperandName) -> void {
+    if (AMDGPU::getNamedOperandIdx(NewOpcode, OperandName) != -1) {
----------------
tsymalla wrote:
> foad wrote:
> > I'd slightly prefer a "getNamedImmOperandOrZero" which just returns the immediate value, instead of calling Builder.add.
> The immediate value should only be added when the operand is found on the instruction, so the builder should only be called with 0 or the immediate value. When just returning the value, it would be required to have additional state which needs to be checked after calling the lambda to ensure a value needs to be added. This would delegate the responsibility back to the caller, but probably complicate everything.
> 
> I agree with renaming the object though.
I refactored this. As getNamedOperand returns nullptr when Idx == -1, the behavior should be the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122489