[PATCH] D40074: [GISel] Canonicalize constants to RHS for commutative operations

[Diana Picus via Phabricator via llvm-commits]

rovka created this revision.
Herald added subscribers: kristof.beyls, igorb, javed.absar, aemerson.

Canonicalize (op imm, reg) to (op reg, imm) in the IRTranslator for
commutative operations.

This makes it easier to match sequences with constants in future passes,
since we don't need to check both possible orders. It is particularly
important for TableGen, since it only generates code to match (op reg,
imm), and there's no point in making it do any extra work to check for
the other variant too.

This change makes it possible for the ARM backend to select e.g.
'ADDri %reg, %imm' for both 'add i32 %reg, %imm' and
'add i32 %imm, %reg', whereas previously for the latter it would
materialize %imm in a register and generate an ADDrr. It should have
similar benefits for the other backends.


https://reviews.llvm.org/D40074

Files:
  lib/CodeGen/GlobalISel/IRTranslator.cpp
  test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll
  test/CodeGen/ARM/GlobalISel/arm-irtranslator.ll


Index: test/CodeGen/ARM/GlobalISel/arm-irtranslator.ll
===================================================================
--- test/CodeGen/ARM/GlobalISel/arm-irtranslator.ll
+++ test/CodeGen/ARM/GlobalISel/arm-irtranslator.ll
@@ -138,6 +138,30 @@
   ret i32 %res
 }
 
+define i32 @test_swap_imm_with_reg(i32 %x) {
+; Add is commutative, so we should see it canonicalized with the immediate
+; on the RHS.
+; CHECK-LABEL: name: test_swap_imm_with_reg
+; CHECK-DAG: [[REG:%[0-9]+]]:_(s32) = COPY %r0
+; CHECK-DAG: [[IMM:%[0-9]+]]:_(s32) = G_CONSTANT i32 16
+; CHECK: G_ADD [[REG]], [[IMM]]
+entry:
+  %res = add i32 16, %x
+  ret i32 %res
+}
+
+define i32 @test_dont_swap_imm_with_reg(i32 %x) {
+; Sub is not commutative, so we should not see it canonicalized with the
+; immediate on the RHS.
+; CHECK-LABEL: name: test_dont_swap_imm_with_reg
+; CHECK-DAG: [[REG:%[0-9]+]]:_(s32) = COPY %r0
+; CHECK-DAG: [[IMM:%[0-9]+]]:_(s32) = G_CONSTANT i32 16
+; CHECK: G_SUB [[IMM]], [[REG]]
+entry:
+  %res = sub i32 16, %x
+  ret i32 %res
+}
+
 define i32 @test_stack_args(i32 %p0, i32 %p1, i32 %p2, i32 %p3, i32 %p4, i32 %p5) {
 ; CHECK-LABEL: name: test_stack_args
 ; CHECK: fixedStack:
Index: test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll
===================================================================
--- test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll
+++ test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll
@@ -345,6 +345,15 @@
   ret i64 %res
 }
 
+; CHECK-LABEL: name: andi64
+; CHECK-DAG: [[ARG1:%[0-9]+]]:_(s64) = COPY %x0
+; CHECK-DAG: [[IMM:%[0-9]+]]:_(s64) = G_CONSTANT i64 1024
+; CHECK: G_AND [[ARG1]], [[IMM]]
+define i64 @andi64imm(i64 %arg1) {
+  %res = and i64 1024, %arg1
+  ret i64 %res
+}
+
 ; CHECK-LABEL: name: andi32
 ; CHECK: [[ARG1:%[0-9]+]]:_(s32) = COPY %w0
 ; CHECK-NEXT: [[ARG2:%[0-9]+]]:_(s32) = COPY %w1
@@ -368,6 +377,15 @@
   ret i64 %res
 }
 
+; CHECK-LABEl: subi64imm
+; CHECK-DAG: [[ARG1:%[0-9]+]]:_(s64) = COPY %x0
+; CHEcK-DAG: [[IMM:%[0-9]+]]:_(s64) = G_CONSTANT i64 128
+; CHECK: G_SUB [[IMM]], [[ARG1]]
+define i64 @subi64imm(i64 %arg1) {
+  %res = sub i64 128, %arg1
+  ret i64 %res
+}
+
 ; CHECK-LABEL: name: subi32
 ; CHECK: [[ARG1:%[0-9]+]]:_(s32) = COPY %w0
 ; CHECK-NEXT: [[ARG2:%[0-9]+]]:_(s32) = COPY %w1
Index: lib/CodeGen/GlobalISel/IRTranslator.cpp
===================================================================
--- lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -196,7 +196,17 @@
   unsigned Op0 = getOrCreateVReg(*U.getOperand(0));
   unsigned Op1 = getOrCreateVReg(*U.getOperand(1));
   unsigned Res = getOrCreateVReg(U);
-  MIRBuilder.buildInstr(Opcode).addDef(Res).addUse(Op0).addUse(Op1);
+
+  bool Op0IsImm = dyn_cast<Constant>(U.getOperand(0));
+  bool Op1IsImm = dyn_cast<Constant>(U.getOperand(1));
+
+  // If the instruction is commutable and only one of the operands is a
+  // constant, canonicalize it to the RHS so it's easier to match by future
+  // passes.
+  auto MI = MIRBuilder.buildInstr(Opcode).addDef(Res);
+  if (MI->isCommutable() && Op0IsImm && !Op1IsImm)
+    std::swap(Op0, Op1);
+  MI.addUse(Op0).addUse(Op1);
   return true;
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D40074.123017.patch
Type: text/x-patch
Size: 3172 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171115/71ecabdf/attachment.bin>