[GlobalISel] Don't combine instructions which are fed by memory instructions using different size

[agostonrobert]

Summary

Memory instructions like extending loads from the same address are not equal if
their size is not equal.

This fixes https://github.com/llvm/llvm-project/issues/53524.

[paquette]

LGTM

[agostonrobert]

In D118805#3291232, @paquette wrote:

LGTM

Thank you for reviewing!
Could you please also commit this for me? I have no commit access yet.

llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp

 //===-- lib/CodeGen/GlobalISel/GICombinerHelper.cpp -----------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 #include "llvm/CodeGen/GlobalISel/CombinerHelper.h"
bool CombinerHelper::matchEqualDefs(const MachineOperand &MOP1,
   //
   // It's possible that @foo will modify whatever lives at the address we're
   // loading from. To be safe, let's just assume that all loads and stores
   // are different (unless we have something which is guaranteed to not
   // change.)
   if (I1->mayLoadOrStore() && !I1->isDereferenceableInvariantLoad(nullptr))
     return false;
 
+  // If both instructions are loads or stores, they are equal only if both
+  // are dereferenceable invariant loads with the same number of bits.
+  if (I1->mayLoadOrStore() && I2->mayLoadOrStore()) {
+    GLoadStore *LS1 = dyn_cast<GLoadStore>(I1);
+    GLoadStore *LS2 = dyn_cast<GLoadStore>(I2);
+    if (!LS1 || !LS2)
+      return false;
+
+    if (!I2->isDereferenceableInvariantLoad(nullptr) ||
+        (LS1->getMemSizeInBits() != LS2->getMemSizeInBits()))
+      return false;
+  }
+
   // Check for physical registers on the instructions first to avoid cases
   // like this:
   //
   // %a = COPY $physreg
   // ...
   // SOMETHING implicit-def $physreg
   // ...
   // %b = COPY $physreg

llvm/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-not-really-equiv-insts.mir

 # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
 # RUN: llc -mtriple aarch64 -run-pass=aarch64-prelegalizer-combiner -verify-machineinstrs %s -o - | FileCheck %s
 
 --- |
   @g = external hidden unnamed_addr global i32, align 4
   define void @not_necessarily_equiv_loads() { ret void }
   define void @invariant_loads() { ret void }
-  define void @both_have_to_be_invariant() { ret void }
+  define void @both_have_to_be_invariant_1() { ret void }
+  define void @both_have_to_be_invariant_2() { ret void }
+  define void @both_have_to_have_same_size() { ret void }
+
 ...
 ---
 name:            not_necessarily_equiv_loads
 tracksRegLiveness: true
 machineFunctionInfo: {}
 body:             |
   bb.0:
 
     ; %load1 || %load2 == %load1 is not necessarily true, even though they
     ; both load from the same address. Whatever is in that address may be
     ; changed by another instruction which appears between them.
     ;
     ; Check that we don't remove the G_OR.
 
     ; CHECK-LABEL: name: not_necessarily_equiv_loads
     ; CHECK: %ptr:_(p0) = G_GLOBAL_VALUE @g
-    ; CHECK: %load1:_(s32) = G_LOAD %ptr(p0) :: (load (s32) from @g)
-    ; CHECK: %load2:_(s32) = G_LOAD %ptr(p0) :: (load (s32) from @g)
-    ; CHECK: %or:_(s32) = G_OR %load2, %load1
-    ; CHECK: G_STORE %or(s32), %ptr(p0) :: (store (s32) into @g)
-    ; CHECK: RET_ReallyLR
+    ; CHECK-NEXT: %load1:_(s32) = G_LOAD %ptr(p0) :: (load (s32) from @g)
+    ; CHECK-NEXT: %load2:_(s32) = G_LOAD %ptr(p0) :: (load (s32) from @g)
+    ; CHECK-NEXT: %or:_(s32) = G_OR %load2, %load1
+    ; CHECK-NEXT: G_STORE %or(s32), %ptr(p0) :: (store (s32) into @g)
+    ; CHECK-NEXT: RET_ReallyLR
     %ptr:_(p0) = G_GLOBAL_VALUE @g
     %load1:_(s32) = G_LOAD %ptr(p0) :: (load (s32) from @g)
     %load2:_(s32) = G_LOAD %ptr(p0) :: (load (s32) from @g)
     %or:_(s32) = G_OR %load2, %load1
     G_STORE %or(s32), %ptr(p0) :: (store (s32) into @g)
     RET_ReallyLR
 
 ...
 ---
 name:            invariant_loads
 tracksRegLiveness: true
 machineFunctionInfo: {}
 body:             |
   bb.0:
 
     ; %load1 || %load2 == %load1 is fine here, because the loads are invariant.
 
     ; CHECK-LABEL: name: invariant_loads
     ; CHECK: %ptr:_(p0) = G_GLOBAL_VALUE @g
-    ; CHECK: %load2:_(s32) = G_LOAD %ptr(p0) :: (dereferenceable invariant load (s32) from @g)
-    ; CHECK: G_STORE %load2(s32), %ptr(p0) :: (store (s32) into @g)
-    ; CHECK: RET_ReallyLR
+    ; CHECK-NEXT: %load2:_(s32) = G_LOAD %ptr(p0) :: (dereferenceable invariant load (s32) from @g)
+    ; CHECK-NEXT: G_STORE %load2(s32), %ptr(p0) :: (store (s32) into @g)
+    ; CHECK-NEXT: RET_ReallyLR
     %ptr:_(p0) = G_GLOBAL_VALUE @g
     %load1:_(s32) = G_LOAD %ptr(p0) :: (dereferenceable invariant load (s32) from @g)
     %load2:_(s32) = G_LOAD %ptr(p0) :: (dereferenceable invariant load (s32) from @g)
     %or:_(s32) = G_OR %load2, %load1
     G_STORE %or(s32), %ptr(p0) :: (store (s32) into @g)
     RET_ReallyLR
 
 ...
 ---
-name:            both_have_to_be_invariant
+name:            both_have_to_be_invariant_1
 tracksRegLiveness: true
 machineFunctionInfo: {}
 body:             |
   bb.0:
 
     ; We shouldn't combine here, because the loads both have to be invariant.
 
-    ; CHECK-LABEL: name: both_have_to_be_invariant
+    ; CHECK-LABEL: name: both_have_to_be_invariant_1
     ; CHECK: %ptr:_(p0) = G_GLOBAL_VALUE @g
-    ; CHECK: %load1:_(s32) = G_LOAD %ptr(p0) :: (dereferenceable invariant load (s32) from @g)
-    ; CHECK: %load2:_(s32) = G_LOAD %ptr(p0) :: (dereferenceable load (s32) from @g)
-    ; CHECK: %or:_(s32) = G_OR %load2, %load1
-    ; CHECK: G_STORE %or(s32), %ptr(p0) :: (store (s32) into @g)
-    ; CHECK: RET_ReallyLR
+    ; CHECK-NEXT: %load1:_(s32) = G_LOAD %ptr(p0) :: (dereferenceable invariant load (s32) from @g)
+    ; CHECK-NEXT: %load2:_(s32) = G_LOAD %ptr(p0) :: (dereferenceable load (s32) from @g)
+    ; CHECK-NEXT: %or:_(s32) = G_OR %load2, %load1
+    ; CHECK-NEXT: G_STORE %or(s32), %ptr(p0) :: (store (s32) into @g)
+    ; CHECK-NEXT: RET_ReallyLR
     %ptr:_(p0) = G_GLOBAL_VALUE @g
     %load1:_(s32) = G_LOAD %ptr(p0) :: (dereferenceable invariant load (s32) from @g)
     %load2:_(s32) = G_LOAD %ptr(p0) :: (dereferenceable load (s32) from @g)
     %or:_(s32) = G_OR %load2, %load1
     G_STORE %or(s32), %ptr(p0) :: (store (s32) into @g)
     RET_ReallyLR
 ...
+---
+name:            both_have_to_be_invariant_2
+tracksRegLiveness: true
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+
+    ; We shouldn't combine here, because the loads both have to be invariant.
+
+    ; CHECK-LABEL: name: both_have_to_be_invariant_2
+    ; CHECK: %ptr:_(p0) = G_GLOBAL_VALUE @g
+    ; CHECK-NEXT: %load1:_(s32) = G_LOAD %ptr(p0) :: (dereferenceable load (s32) from @g)
+    ; CHECK-NEXT: %load2:_(s32) = G_LOAD %ptr(p0) :: (dereferenceable invariant load (s32) from @g)
+    ; CHECK-NEXT: %or:_(s32) = G_OR %load2, %load1
+    ; CHECK-NEXT: G_STORE %or(s32), %ptr(p0) :: (store (s32) into @g)
+    ; CHECK-NEXT: RET_ReallyLR
+    %ptr:_(p0) = G_GLOBAL_VALUE @g
+    %load1:_(s32) = G_LOAD %ptr(p0) :: (dereferenceable load (s32) from @g)
+    %load2:_(s32) = G_LOAD %ptr(p0) :: (dereferenceable invariant load (s32) from @g)
+    %or:_(s32) = G_OR %load2, %load1
+    G_STORE %or(s32), %ptr(p0) :: (store (s32) into @g)
+    RET_ReallyLR
+...
+---
+name:            both_have_to_have_same_size
+tracksRegLiveness: true
+machineFunctionInfo: {}
+body:             |
+  bb.0:
+
+    ; We shouldn't combine here, because the loads both have to have the same size.
+
+    ; CHECK-LABEL: name: both_have_to_have_same_size
+    ; CHECK: %ptr:_(p0) = G_GLOBAL_VALUE @g
+    ; CHECK-NEXT: %load1:_(s32) = G_ZEXTLOAD %ptr(p0) :: (dereferenceable invariant load (s8) from @g)
+    ; CHECK-NEXT: %load2:_(s32) = G_ZEXTLOAD %ptr(p0) :: (dereferenceable invariant load (s16) from @g)
+    ; CHECK-NEXT: %or:_(s32) = G_OR %load2, %load1
+    ; CHECK-NEXT: G_STORE %or(s32), %ptr(p0) :: (store (s32) into @g)
+    ; CHECK-NEXT: RET_ReallyLR
+    %ptr:_(p0) = G_GLOBAL_VALUE @g
+    %load1:_(s32) = G_ZEXTLOAD %ptr(p0) :: (dereferenceable invariant load (s8) from @g)
+    %load2:_(s32) = G_ZEXTLOAD %ptr(p0) :: (dereferenceable invariant load (s16) from @g)
+    %or:_(s32) = G_OR %load2, %load1
+    G_STORE %or(s32), %ptr(p0) :: (store (s32) into @g)
+    RET_ReallyLR
+...