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

[Thomas Symalla via Phabricator via llvm-commits]

tsymalla marked an inline comment as done.
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) {
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122489