[PATCH] D140907: [GlobalISel] New combine to commute constant operands to the RHS

[Thomas Symalla via Phabricator via llvm-commits]

tsymalla added a comment.

LGTM



================
Comment at: llvm/include/llvm/Target/GlobalISel/Combine.td:349
+  (match (wip_match_opcode G_ADD, G_MUL, G_AND, G_OR, G_XOR):$root, [{
+    return getIConstantVRegVal(${root}->getOperand(1).getReg(), MRI).has_value();
+  }]),
----------------
This looks alright to me, but what is the point in swapping the operands if both of them are constants except making the ISA more readable?
For instance:

`s_add_i32 s1, 0x1000, 0 => s_add_i32 s1, 0, 0x1000`


================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/mul-known-bits.i64.ll:174
 ; GFX11-NEXT:    v_lshlrev_b32_e32 v0, 2, v0
+; GFX11-NEXT:    v_mov_b32_e32 v2, 0
 ; GFX11-NEXT:    s_waitcnt lgkmcnt(0)
----------------
Shouldn't this one be eliminated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140907