]> granicus.if.org Git - llvm/commitdiff
[AArch64][GlobalISel] Fix the G_EXTLOAD combiner creating non-extending illegal instr...
authorAmara Emerson <aemerson@apple.com>
Sun, 27 Jan 2019 10:56:20 +0000 (10:56 +0000)
committerAmara Emerson <aemerson@apple.com>
Sun, 27 Jan 2019 10:56:20 +0000 (10:56 +0000)
This fixes loads like 's1 = load %p (load 1 from %p)' being combined with an
extend into an illegal 's8 = g_extload %p (load 1 from %p)' which doesn't do any
extension, by avoiding touching those < s8 size loads.

This bug was uncovered by a verifier update r351584, which I reverted it to keep
the bots green.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@352311 91177308-0d34-0410-b5e6-96231b3b80d8

lib/CodeGen/GlobalISel/CombinerHelper.cpp
test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads-s1.mir [new file with mode: 0644]
test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads.mir

index cff637488cf4e357abb50e95687a3755ea58b474..a2b1e8e1eb4afed0182f3b8122497e850c21d4e9 100644 (file)
@@ -181,6 +181,14 @@ bool CombinerHelper::tryCombineExtendingLoads(MachineInstr &MI) {
   if (!LoadValueTy.isScalar())
     return false;
 
+  // Most architectures are going to legalize <s8 loads into at least a 1 byte
+  // load, and the MMOs can only describe memory accesses in multiples of bytes.
+  // If we try to perform extload combining on those, we can end up with
+  // %a(s8) = extload %ptr (load 1 byte from %ptr)
+  // ... which is an illegal extload instruction.
+  if (LoadValueTy.getSizeInBits() < 8)
+    return false;
+
   // Find the preferred type aside from the any-extends (unless it's the only
   // one) and non-extending ops. We'll emit an extending load to that type and
   // and emit a variant of (extend (trunc X)) for the others according to the
diff --git a/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads-s1.mir b/test/CodeGen/AArch64/GlobalISel/prelegalizercombiner-extending-loads-s1.mir
new file mode 100644 (file)
index 0000000..e8decdc
--- /dev/null
@@ -0,0 +1,39 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple aarch64 -O0 -run-pass=aarch64-prelegalizer-combiner -global-isel -verify-machineinstrs %s -o - | FileCheck %s
+
+# Check we don't try to combine a load of < s8 as that will end up creating a illegal non-extending load.
+--- |
+  define i8 @test(i1* %ptr) {
+    ret i8 undef
+  }
+
+...
+---
+name:            test
+alignment:       2
+tracksRegLiveness: true
+registers:
+  - { id: 0, class: _ }
+  - { id: 1, class: _ }
+  - { id: 2, class: _ }
+  - { id: 3, class: _ }
+body:             |
+  bb.1 (%ir-block.0):
+    liveins: $x0
+
+    ; CHECK-LABEL: name: test
+    ; CHECK: liveins: $x0
+    ; CHECK: [[COPY:%[0-9]+]]:_(p0) = COPY $x0
+    ; CHECK: [[LOAD:%[0-9]+]]:_(s1) = G_LOAD [[COPY]](p0) :: (load 1 from %ir.ptr)
+    ; CHECK: [[ZEXT:%[0-9]+]]:_(s8) = G_ZEXT [[LOAD]](s1)
+    ; CHECK: [[ANYEXT:%[0-9]+]]:_(s32) = G_ANYEXT [[ZEXT]](s8)
+    ; CHECK: $w0 = COPY [[ANYEXT]](s32)
+    ; CHECK: RET_ReallyLR implicit $w0
+    %0:_(p0) = COPY $x0
+    %1:_(s1) = G_LOAD %0(p0) :: (load 1 from %ir.ptr)
+    %2:_(s8) = G_ZEXT %1(s1)
+    %3:_(s32) = G_ANYEXT %2(s8)
+    $w0 = COPY %3(s32)
+    RET_ReallyLR implicit $w0
+
+...
index 47d7156789f8d7c2458643696ee58a3f783ddb9b..c8360ad59d4d4852c211402de0c0e3edf871183c 100644 (file)
@@ -1,4 +1,4 @@
-# RUN: llc -O0 -run-pass=aarch64-prelegalizer-combiner -global-isel %s -o - | FileCheck %s
+# RUN: llc -O0 -run-pass=aarch64-prelegalizer-combiner -global-isel -verify-machineinstrs %s -o - | FileCheck %s
 
 --- |
   target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"