[PATCH] D114655: Make G_PTR_ADD pattern matcher non-commutative.

[Daniel Thornburgh via Phabricator via llvm-commits]

mysterymath created this revision.
mysterymath added a reviewer: aemerson.
Herald added subscribers: kristof.beyls, arichardson.
mysterymath requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

G_PTR_ADD takes arguments of two different types, so it probably shouldn't be
considered commutative just on that basis. A recent G_PTR_ADD reassociation
optimization (https://reviews.llvm.org/D109528) can emit erroneous code if the
pattern matcher commutes the arguments; this can happen when the base pointer
was created by G_INTTOPTR of a G_CONSTANT and the offset register is variable.

This was discovered on the llvm-mos fork, but I added a failing test case that
should apply to AArch64 (and more generally).

Also, this is my first change to LLVM proper, so please let me know if there's
anything amiss with my interpretation of the CR process. Take care!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114655

Files:
  llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h
  llvm/test/CodeGen/AArch64/GlobalISel/combine-ptradd-reassociation.mir


Index: llvm/test/CodeGen/AArch64/GlobalISel/combine-ptradd-reassociation.mir
===================================================================
--- llvm/test/CodeGen/AArch64/GlobalISel/combine-ptradd-reassociation.mir
+++ llvm/test/CodeGen/AArch64/GlobalISel/combine-ptradd-reassociation.mir
@@ -263,3 +263,44 @@
     RET_ReallyLR
 
 ...
+---
+name:            reassoc_cst_inner_lhs_const_lhs
+alignment:       4
+tracksRegLiveness: true
+liveins:
+  - { reg: '$w0' }
+  - { reg: '$x1' }
+  - { reg: '$x2' }
+  - { reg: '$x3' }
+body:             |
+  bb.1:
+    liveins: $w0, $x1, $x2, $x3
+
+    ; CHECK-LABEL: name: reassoc_cst_inner_lhs_const_lhs
+    ; CHECK: liveins: $w0, $x1, $x2, $x3
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:_(s64) = COPY $x2
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:_(s64) = COPY $x3
+    ; CHECK-NEXT: [[C:%[0-9]+]]:_(s64) = G_CONSTANT i64 40
+    ; CHECK-NEXT: [[INTTOPTR:%[0-9]+]]:_(p0) = G_INTTOPTR [[C]](s64)
+    ; CHECK-NEXT: [[PTR_ADD:%[0-9]+]]:_(p0) = G_PTR_ADD [[INTTOPTR]], [[COPY]](s64)
+    ; CHECK-NEXT: [[C1:%[0-9]+]]:_(s64) = G_CONSTANT i64 2
+    ; CHECK-NEXT: [[SHL:%[0-9]+]]:_(s64) = G_SHL [[COPY1]], [[C1]](s64)
+    ; CHECK-NEXT: [[PTR_ADD1:%[0-9]+]]:_(p0) = G_PTR_ADD [[PTR_ADD]], [[SHL]](s64)
+    ; CHECK-NEXT: [[LOAD:%[0-9]+]]:_(s32) = G_LOAD [[PTR_ADD1]](p0) :: (load (s32))
+    ; CHECK-NEXT: $w0 = COPY [[LOAD]](s32)
+    ; CHECK-NEXT: RET_ReallyLR
+    %1:_(p0) = COPY $x1
+    %2:_(s64) = COPY $x2
+    %3:_(s64) = COPY $x3
+    %8:_(s64) = G_CONSTANT i64 40
+    %9:_(p0) = G_INTTOPTR %8(s64)
+    %10:_(p0) = G_PTR_ADD %9, %2(s64)
+    %11:_(s64) = G_CONSTANT i64 2
+    %12:_(s64) = G_SHL %3, %11
+    %13:_(p0) = G_PTR_ADD %10, %12(s64)
+    %15:_(s32) = G_LOAD %13(p0) :: (load (s32))
+    $w0 = COPY %15
+    RET_ReallyLR
+
+...
Index: llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h
===================================================================
--- llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h
+++ llvm/include/llvm/CodeGen/GlobalISel/MIPatternMatch.h
@@ -324,9 +324,9 @@
 }
 
 template <typename LHS, typename RHS>
-inline BinaryOp_match<LHS, RHS, TargetOpcode::G_PTR_ADD, true>
+inline BinaryOp_match<LHS, RHS, TargetOpcode::G_PTR_ADD, false>
 m_GPtrAdd(const LHS &L, const RHS &R) {
-  return BinaryOp_match<LHS, RHS, TargetOpcode::G_PTR_ADD, true>(L, R);
+  return BinaryOp_match<LHS, RHS, TargetOpcode::G_PTR_ADD, false>(L, R);
 }
 
 template <typename LHS, typename RHS>


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D114655.390105.patch
Type: text/x-patch
Size: 2481 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211126/79f64bd6/attachment.bin>