Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[AArch64][GlobalISel] Look into array's element #74109

Merged
merged 3 commits into from
Dec 15, 2023
Merged

[AArch64][GlobalISel] Look into array's element #74109

merged 3 commits into from
Dec 15, 2023

Conversation

chuongg3
Copy link
Contributor

@chuongg3 chuongg3 commented Dec 1, 2023

In AArch64RegisterBankInfo, IsFPOrFPType() does not work correctly
with ArrayTypes and StructTypes as it does not not look at their elements.

This caused some registers to be selected as gpr instead of fpr.

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 1, 2023

@llvm/pr-subscribers-llvm-globalisel

Author: None (chuongg3)

Changes

In AArch64RegisterBankInfo, IsFPOrFPType() does not work correctly
with ArrayTypes and StructTypes as it does not not look at their elements.

This caused some registers to be selected as gpr instead of fpr.

Full diff: https://github.com/llvm/llvm-project/pull/74109.diff

2 Files Affected:

  • (modified) llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp (+11)
  • (modified) llvm/test/CodeGen/AArch64/GlobalISel/regbankselect-fp-loads.mir (+295-26)
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
index 3284d0f033e3b44..263a9f1cec8736a 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
@@ -621,6 +621,17 @@ bool AArch64RegisterBankInfo::isLoadFromFPType(const MachineInstr &MI) const {
   Type *EltTy = nullptr;
   if (const GlobalValue *GV = dyn_cast<GlobalValue>(LdVal)) {
     EltTy = GV->getValueType();
+    // Look at the first element of the struct to determine its type
+    if (StructType *StructEltTy = dyn_cast<StructType>(EltTy)) {
+      while (isa<StructType>(StructEltTy->getTypeAtIndex(0U))) {
+        StructEltTy = dyn_cast<StructType>(StructEltTy->getTypeAtIndex(0U));
+      }
+      EltTy = StructEltTy->getTypeAtIndex(0U);
+    }
+    // Look at the first element of the array to determine its type
+    if (isa<ArrayType>(EltTy)) {
+      EltTy = EltTy->getArrayElementType();
+    }
   } else {
     // FIXME: grubbing around uses is pretty ugly, but with no more
     // `getPointerElementType` there's not much else we can do.
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/regbankselect-fp-loads.mir b/llvm/test/CodeGen/AArch64/GlobalISel/regbankselect-fp-loads.mir
index cfa0ba90c5875d3..6efa6f29d850af4 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/regbankselect-fp-loads.mir
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/regbankselect-fp-loads.mir
@@ -10,6 +10,23 @@
   define float @fp_load_phi() { ret float undef }
   define i32 @int_load_phi() { ret i32 undef }
 
+
+  @array_double = dso_local global [32000 x double] zeroinitializer, align 8
+  @struct_array_double = dso_local global { [32000 x double] } zeroinitializer, align 8
+  @struct_struct_array_double = dso_local global {{ [32000 x double] }} zeroinitializer, align 8
+
+  define float @array_load_double() { ret float undef }
+  define float @struct_array_load_double() { ret float undef }
+  define float @struct_struct_array_load_double() { ret float undef }
+
+  @array_int = dso_local global [32000 x i32] zeroinitializer, align 8
+  @struct_array_int = dso_local global { [32000 x i32] } zeroinitializer, align 8
+  @struct_struct_array_int = dso_local global {{ [32000 x i32] }} zeroinitializer, align 8
+
+  define i32 @array_load_int() { ret i32 undef }
+  define i32 @struct_array_load_int() { ret i32 undef }
+  define i32 @struct_struct_array_load_int() { ret i32 undef }
+
 ...
 ---
 name:            fp_load_phi
@@ -19,20 +36,22 @@ tracksRegLiveness: true
 body:             |
   ; CHECK-LABEL: name: fp_load_phi
   ; CHECK: bb.0:
-  ; CHECK:   successors: %bb.1(0x80000000)
-  ; CHECK:   liveins: $w0
-  ; CHECK:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
-  ; CHECK:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @var_fp
-  ; CHECK:   %fp_load:fpr(s32) = G_LOAD [[GV]](p0) :: (load (s32) from @var_fp)
-  ; CHECK: bb.1:
-  ; CHECK:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
-  ; CHECK:   [[PHI:%[0-9]+]]:fpr(s32) = PHI %fp_load(s32), %bb.0, [[PHI]](s32), %bb.1
-  ; CHECK:   G_BRCOND [[COPY]](s32), %bb.1
-  ; CHECK: bb.2:
-  ; CHECK:   $s0 = COPY [[PHI]](s32)
-  ; CHECK:   RET_ReallyLR implicit $s0
-  ; Here we're checking that the load is assigned an FPR bank, since it's
-  ; loading from an fp type in the IR.
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
+  ; CHECK-NEXT:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @var_fp
+  ; CHECK-NEXT:   %fp_load:fpr(s32) = G_LOAD [[GV]](p0) :: (load (s32) from @var_fp)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[PHI:%[0-9]+]]:fpr(s32) = PHI %fp_load(s32), %bb.0, [[PHI]](s32), %bb.1
+  ; CHECK-NEXT:   G_BRCOND [[COPY]](s32), %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   $s0 = COPY [[PHI]](s32)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $s0
   bb.0:
     liveins: $w0
     successors: %bb.1
@@ -58,18 +77,22 @@ tracksRegLiveness: true
 body:             |
   ; CHECK-LABEL: name: int_load_phi
   ; CHECK: bb.0:
-  ; CHECK:   successors: %bb.1(0x80000000)
-  ; CHECK:   liveins: $w0
-  ; CHECK:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
-  ; CHECK:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @var_fp
-  ; CHECK:   %fp_load:gpr(s32) = G_LOAD [[GV]](p0) :: (load (s32) from @var_int)
-  ; CHECK: bb.1:
-  ; CHECK:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
-  ; CHECK:   [[PHI:%[0-9]+]]:gpr(s32) = PHI %fp_load(s32), %bb.0, [[PHI]](s32), %bb.1
-  ; CHECK:   G_BRCOND [[COPY]](s32), %bb.1
-  ; CHECK: bb.2:
-  ; CHECK:   $s0 = COPY [[PHI]](s32)
-  ; CHECK:   RET_ReallyLR implicit $s0
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
+  ; CHECK-NEXT:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @var_fp
+  ; CHECK-NEXT:   %fp_load:gpr(s32) = G_LOAD [[GV]](p0) :: (load (s32) from @var_int)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[PHI:%[0-9]+]]:gpr(s32) = PHI %fp_load(s32), %bb.0, [[PHI]](s32), %bb.1
+  ; CHECK-NEXT:   G_BRCOND [[COPY]](s32), %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   $s0 = COPY [[PHI]](s32)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $s0
   bb.0:
     liveins: $w0
     successors: %bb.1
@@ -86,3 +109,249 @@ body:             |
     $s0 = COPY %2
     RET_ReallyLR implicit $s0
 ...
+
+---
+name:            array_load_double
+legalized:       true
+regBankSelected: false
+tracksRegLiveness: true
+body:             |
+  ; CHECK-LABEL: name: array_load_double
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
+  ; CHECK-NEXT:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @array_double
+  ; CHECK-NEXT:   %fp_load:fpr(s64) = G_LOAD [[GV]](p0) :: (dereferenceable load (s64) from @array_double)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[PHI:%[0-9]+]]:fpr(s32) = PHI %fp_load(s64), %bb.0, [[PHI]](s32), %bb.1
+  ; CHECK-NEXT:   G_BRCOND [[COPY]](s32), %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   $s0 = COPY [[PHI]](s32)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $s0
+  bb.0:
+    liveins: $w0
+    successors: %bb.1
+    %0:_(s32) = COPY $w0
+    %1:_(p0) = G_GLOBAL_VALUE @array_double
+    %fp_load:_(s64) = G_LOAD %1(p0) :: (dereferenceable load (s64) from @array_double)
+
+  bb.1:
+    successors: %bb.1, %bb.2
+    %2:_(s32) = PHI %fp_load, %bb.0, %2, %bb.1
+    G_BRCOND %0, %bb.1
+
+  bb.2:
+    $s0 = COPY %2
+    RET_ReallyLR implicit $s0
+...
+
+---
+name:            struct_array_load_double
+legalized:       true
+regBankSelected: false
+tracksRegLiveness: true
+body:             |
+  ; CHECK-LABEL: name: struct_array_load_double
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
+  ; CHECK-NEXT:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @struct_array_double
+  ; CHECK-NEXT:   %fp_load:fpr(s64) = G_LOAD [[GV]](p0) :: (dereferenceable load (s64) from @struct_array_double)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[PHI:%[0-9]+]]:fpr(s32) = PHI %fp_load(s64), %bb.0, [[PHI]](s32), %bb.1
+  ; CHECK-NEXT:   G_BRCOND [[COPY]](s32), %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   $s0 = COPY [[PHI]](s32)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $s0
+  bb.0:
+    liveins: $w0
+    successors: %bb.1
+    %0:_(s32) = COPY $w0
+    %1:_(p0) = G_GLOBAL_VALUE @struct_array_double
+    %fp_load:_(s64) = G_LOAD %1(p0) :: (dereferenceable load (s64) from @struct_array_double)
+
+  bb.1:
+    successors: %bb.1, %bb.2
+    %2:_(s32) = PHI %fp_load, %bb.0, %2, %bb.1
+    G_BRCOND %0, %bb.1
+
+  bb.2:
+    $s0 = COPY %2
+    RET_ReallyLR implicit $s0
+...
+
+---
+name:            struct_struct_array_load_double
+legalized:       true
+regBankSelected: false
+tracksRegLiveness: true
+body:             |
+  ; CHECK-LABEL: name: struct_struct_array_load_double
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
+  ; CHECK-NEXT:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @struct_struct_array_double
+  ; CHECK-NEXT:   %fp_load:fpr(s64) = G_LOAD [[GV]](p0) :: (dereferenceable load (s64) from @struct_struct_array_double)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[PHI:%[0-9]+]]:fpr(s32) = PHI %fp_load(s64), %bb.0, [[PHI]](s32), %bb.1
+  ; CHECK-NEXT:   G_BRCOND [[COPY]](s32), %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   $s0 = COPY [[PHI]](s32)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $s0
+  bb.0:
+    liveins: $w0
+    successors: %bb.1
+    %0:_(s32) = COPY $w0
+    %1:_(p0) = G_GLOBAL_VALUE @struct_struct_array_double
+    %fp_load:_(s64) = G_LOAD %1(p0) :: (dereferenceable load (s64) from @struct_struct_array_double)
+
+  bb.1:
+    successors: %bb.1, %bb.2
+    %2:_(s32) = PHI %fp_load, %bb.0, %2, %bb.1
+    G_BRCOND %0, %bb.1
+
+  bb.2:
+    $s0 = COPY %2
+    RET_ReallyLR implicit $s0
+...
+
+---
+name:            array_load_int
+legalized:       true
+regBankSelected: false
+tracksRegLiveness: true
+body:             |
+  ; CHECK-LABEL: name: array_load_int
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
+  ; CHECK-NEXT:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @array_int
+  ; CHECK-NEXT:   %fp_load:gpr(s64) = G_LOAD [[GV]](p0) :: (dereferenceable load (s64) from @array_int)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[PHI:%[0-9]+]]:gpr(s32) = PHI %fp_load(s64), %bb.0, [[PHI]](s32), %bb.1
+  ; CHECK-NEXT:   G_BRCOND [[COPY]](s32), %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   $s0 = COPY [[PHI]](s32)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $s0
+  bb.0:
+    liveins: $w0
+    successors: %bb.1
+    %0:_(s32) = COPY $w0
+    %1:_(p0) = G_GLOBAL_VALUE @array_int
+    %fp_load:_(s64) = G_LOAD %1(p0) :: (dereferenceable load (s64) from @array_int)
+
+  bb.1:
+    successors: %bb.1, %bb.2
+    %2:_(s32) = PHI %fp_load, %bb.0, %2, %bb.1
+    G_BRCOND %0, %bb.1
+
+  bb.2:
+    $s0 = COPY %2
+    RET_ReallyLR implicit $s0
+...
+
+---
+name:            struct_array_load_int
+legalized:       true
+regBankSelected: false
+tracksRegLiveness: true
+body:             |
+  ; CHECK-LABEL: name: struct_array_load_int
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
+  ; CHECK-NEXT:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @struct_array_int
+  ; CHECK-NEXT:   %fp_load:gpr(s64) = G_LOAD [[GV]](p0) :: (dereferenceable load (s64) from @struct_array_int)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[PHI:%[0-9]+]]:gpr(s32) = PHI %fp_load(s64), %bb.0, [[PHI]](s32), %bb.1
+  ; CHECK-NEXT:   G_BRCOND [[COPY]](s32), %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   $s0 = COPY [[PHI]](s32)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $s0
+  bb.0:
+    liveins: $w0
+    successors: %bb.1
+    %0:_(s32) = COPY $w0
+    %1:_(p0) = G_GLOBAL_VALUE @struct_array_int
+    %fp_load:_(s64) = G_LOAD %1(p0) :: (dereferenceable load (s64) from @struct_array_int)
+
+  bb.1:
+    successors: %bb.1, %bb.2
+    %2:_(s32) = PHI %fp_load, %bb.0, %2, %bb.1
+    G_BRCOND %0, %bb.1
+
+  bb.2:
+    $s0 = COPY %2
+    RET_ReallyLR implicit $s0
+...
+
+---
+name:            struct_struct_array_load_int
+legalized:       true
+regBankSelected: false
+tracksRegLiveness: true
+body:             |
+  ; CHECK-LABEL: name: struct_struct_array_load_int
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
+  ; CHECK-NEXT:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @struct_struct_array_int
+  ; CHECK-NEXT:   %fp_load:gpr(s64) = G_LOAD [[GV]](p0) :: (dereferenceable load (s64) from @struct_struct_array_int)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[PHI:%[0-9]+]]:gpr(s32) = PHI %fp_load(s64), %bb.0, [[PHI]](s32), %bb.1
+  ; CHECK-NEXT:   G_BRCOND [[COPY]](s32), %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   $s0 = COPY [[PHI]](s32)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $s0
+  bb.0:
+    liveins: $w0
+    successors: %bb.1
+    %0:_(s32) = COPY $w0
+    %1:_(p0) = G_GLOBAL_VALUE @struct_struct_array_int
+    %fp_load:_(s64) = G_LOAD %1(p0) :: (dereferenceable load (s64) from @struct_struct_array_int)
+
+  bb.1:
+    successors: %bb.1, %bb.2
+    %2:_(s32) = PHI %fp_load, %bb.0, %2, %bb.1
+    G_BRCOND %0, %bb.1
+
+  bb.2:
+    $s0 = COPY %2
+    RET_ReallyLR implicit $s0
+...

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 1, 2023

@llvm/pr-subscribers-backend-aarch64

Author: None (chuongg3)

Changes

In AArch64RegisterBankInfo, IsFPOrFPType() does not work correctly
with ArrayTypes and StructTypes as it does not not look at their elements.

This caused some registers to be selected as gpr instead of fpr.

Full diff: https://github.com/llvm/llvm-project/pull/74109.diff

2 Files Affected:

  • (modified) llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp (+11)
  • (modified) llvm/test/CodeGen/AArch64/GlobalISel/regbankselect-fp-loads.mir (+295-26)
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
index 3284d0f033e3b44..263a9f1cec8736a 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
@@ -621,6 +621,17 @@ bool AArch64RegisterBankInfo::isLoadFromFPType(const MachineInstr &MI) const {
   Type *EltTy = nullptr;
   if (const GlobalValue *GV = dyn_cast<GlobalValue>(LdVal)) {
     EltTy = GV->getValueType();
+    // Look at the first element of the struct to determine its type
+    if (StructType *StructEltTy = dyn_cast<StructType>(EltTy)) {
+      while (isa<StructType>(StructEltTy->getTypeAtIndex(0U))) {
+        StructEltTy = dyn_cast<StructType>(StructEltTy->getTypeAtIndex(0U));
+      }
+      EltTy = StructEltTy->getTypeAtIndex(0U);
+    }
+    // Look at the first element of the array to determine its type
+    if (isa<ArrayType>(EltTy)) {
+      EltTy = EltTy->getArrayElementType();
+    }
   } else {
     // FIXME: grubbing around uses is pretty ugly, but with no more
     // `getPointerElementType` there's not much else we can do.
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/regbankselect-fp-loads.mir b/llvm/test/CodeGen/AArch64/GlobalISel/regbankselect-fp-loads.mir
index cfa0ba90c5875d3..6efa6f29d850af4 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/regbankselect-fp-loads.mir
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/regbankselect-fp-loads.mir
@@ -10,6 +10,23 @@
   define float @fp_load_phi() { ret float undef }
   define i32 @int_load_phi() { ret i32 undef }
 
+
+  @array_double = dso_local global [32000 x double] zeroinitializer, align 8
+  @struct_array_double = dso_local global { [32000 x double] } zeroinitializer, align 8
+  @struct_struct_array_double = dso_local global {{ [32000 x double] }} zeroinitializer, align 8
+
+  define float @array_load_double() { ret float undef }
+  define float @struct_array_load_double() { ret float undef }
+  define float @struct_struct_array_load_double() { ret float undef }
+
+  @array_int = dso_local global [32000 x i32] zeroinitializer, align 8
+  @struct_array_int = dso_local global { [32000 x i32] } zeroinitializer, align 8
+  @struct_struct_array_int = dso_local global {{ [32000 x i32] }} zeroinitializer, align 8
+
+  define i32 @array_load_int() { ret i32 undef }
+  define i32 @struct_array_load_int() { ret i32 undef }
+  define i32 @struct_struct_array_load_int() { ret i32 undef }
+
 ...
 ---
 name:            fp_load_phi
@@ -19,20 +36,22 @@ tracksRegLiveness: true
 body:             |
   ; CHECK-LABEL: name: fp_load_phi
   ; CHECK: bb.0:
-  ; CHECK:   successors: %bb.1(0x80000000)
-  ; CHECK:   liveins: $w0
-  ; CHECK:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
-  ; CHECK:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @var_fp
-  ; CHECK:   %fp_load:fpr(s32) = G_LOAD [[GV]](p0) :: (load (s32) from @var_fp)
-  ; CHECK: bb.1:
-  ; CHECK:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
-  ; CHECK:   [[PHI:%[0-9]+]]:fpr(s32) = PHI %fp_load(s32), %bb.0, [[PHI]](s32), %bb.1
-  ; CHECK:   G_BRCOND [[COPY]](s32), %bb.1
-  ; CHECK: bb.2:
-  ; CHECK:   $s0 = COPY [[PHI]](s32)
-  ; CHECK:   RET_ReallyLR implicit $s0
-  ; Here we're checking that the load is assigned an FPR bank, since it's
-  ; loading from an fp type in the IR.
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
+  ; CHECK-NEXT:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @var_fp
+  ; CHECK-NEXT:   %fp_load:fpr(s32) = G_LOAD [[GV]](p0) :: (load (s32) from @var_fp)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[PHI:%[0-9]+]]:fpr(s32) = PHI %fp_load(s32), %bb.0, [[PHI]](s32), %bb.1
+  ; CHECK-NEXT:   G_BRCOND [[COPY]](s32), %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   $s0 = COPY [[PHI]](s32)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $s0
   bb.0:
     liveins: $w0
     successors: %bb.1
@@ -58,18 +77,22 @@ tracksRegLiveness: true
 body:             |
   ; CHECK-LABEL: name: int_load_phi
   ; CHECK: bb.0:
-  ; CHECK:   successors: %bb.1(0x80000000)
-  ; CHECK:   liveins: $w0
-  ; CHECK:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
-  ; CHECK:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @var_fp
-  ; CHECK:   %fp_load:gpr(s32) = G_LOAD [[GV]](p0) :: (load (s32) from @var_int)
-  ; CHECK: bb.1:
-  ; CHECK:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
-  ; CHECK:   [[PHI:%[0-9]+]]:gpr(s32) = PHI %fp_load(s32), %bb.0, [[PHI]](s32), %bb.1
-  ; CHECK:   G_BRCOND [[COPY]](s32), %bb.1
-  ; CHECK: bb.2:
-  ; CHECK:   $s0 = COPY [[PHI]](s32)
-  ; CHECK:   RET_ReallyLR implicit $s0
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
+  ; CHECK-NEXT:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @var_fp
+  ; CHECK-NEXT:   %fp_load:gpr(s32) = G_LOAD [[GV]](p0) :: (load (s32) from @var_int)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[PHI:%[0-9]+]]:gpr(s32) = PHI %fp_load(s32), %bb.0, [[PHI]](s32), %bb.1
+  ; CHECK-NEXT:   G_BRCOND [[COPY]](s32), %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   $s0 = COPY [[PHI]](s32)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $s0
   bb.0:
     liveins: $w0
     successors: %bb.1
@@ -86,3 +109,249 @@ body:             |
     $s0 = COPY %2
     RET_ReallyLR implicit $s0
 ...
+
+---
+name:            array_load_double
+legalized:       true
+regBankSelected: false
+tracksRegLiveness: true
+body:             |
+  ; CHECK-LABEL: name: array_load_double
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
+  ; CHECK-NEXT:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @array_double
+  ; CHECK-NEXT:   %fp_load:fpr(s64) = G_LOAD [[GV]](p0) :: (dereferenceable load (s64) from @array_double)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[PHI:%[0-9]+]]:fpr(s32) = PHI %fp_load(s64), %bb.0, [[PHI]](s32), %bb.1
+  ; CHECK-NEXT:   G_BRCOND [[COPY]](s32), %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   $s0 = COPY [[PHI]](s32)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $s0
+  bb.0:
+    liveins: $w0
+    successors: %bb.1
+    %0:_(s32) = COPY $w0
+    %1:_(p0) = G_GLOBAL_VALUE @array_double
+    %fp_load:_(s64) = G_LOAD %1(p0) :: (dereferenceable load (s64) from @array_double)
+
+  bb.1:
+    successors: %bb.1, %bb.2
+    %2:_(s32) = PHI %fp_load, %bb.0, %2, %bb.1
+    G_BRCOND %0, %bb.1
+
+  bb.2:
+    $s0 = COPY %2
+    RET_ReallyLR implicit $s0
+...
+
+---
+name:            struct_array_load_double
+legalized:       true
+regBankSelected: false
+tracksRegLiveness: true
+body:             |
+  ; CHECK-LABEL: name: struct_array_load_double
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
+  ; CHECK-NEXT:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @struct_array_double
+  ; CHECK-NEXT:   %fp_load:fpr(s64) = G_LOAD [[GV]](p0) :: (dereferenceable load (s64) from @struct_array_double)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[PHI:%[0-9]+]]:fpr(s32) = PHI %fp_load(s64), %bb.0, [[PHI]](s32), %bb.1
+  ; CHECK-NEXT:   G_BRCOND [[COPY]](s32), %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   $s0 = COPY [[PHI]](s32)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $s0
+  bb.0:
+    liveins: $w0
+    successors: %bb.1
+    %0:_(s32) = COPY $w0
+    %1:_(p0) = G_GLOBAL_VALUE @struct_array_double
+    %fp_load:_(s64) = G_LOAD %1(p0) :: (dereferenceable load (s64) from @struct_array_double)
+
+  bb.1:
+    successors: %bb.1, %bb.2
+    %2:_(s32) = PHI %fp_load, %bb.0, %2, %bb.1
+    G_BRCOND %0, %bb.1
+
+  bb.2:
+    $s0 = COPY %2
+    RET_ReallyLR implicit $s0
+...
+
+---
+name:            struct_struct_array_load_double
+legalized:       true
+regBankSelected: false
+tracksRegLiveness: true
+body:             |
+  ; CHECK-LABEL: name: struct_struct_array_load_double
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
+  ; CHECK-NEXT:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @struct_struct_array_double
+  ; CHECK-NEXT:   %fp_load:fpr(s64) = G_LOAD [[GV]](p0) :: (dereferenceable load (s64) from @struct_struct_array_double)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[PHI:%[0-9]+]]:fpr(s32) = PHI %fp_load(s64), %bb.0, [[PHI]](s32), %bb.1
+  ; CHECK-NEXT:   G_BRCOND [[COPY]](s32), %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   $s0 = COPY [[PHI]](s32)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $s0
+  bb.0:
+    liveins: $w0
+    successors: %bb.1
+    %0:_(s32) = COPY $w0
+    %1:_(p0) = G_GLOBAL_VALUE @struct_struct_array_double
+    %fp_load:_(s64) = G_LOAD %1(p0) :: (dereferenceable load (s64) from @struct_struct_array_double)
+
+  bb.1:
+    successors: %bb.1, %bb.2
+    %2:_(s32) = PHI %fp_load, %bb.0, %2, %bb.1
+    G_BRCOND %0, %bb.1
+
+  bb.2:
+    $s0 = COPY %2
+    RET_ReallyLR implicit $s0
+...
+
+---
+name:            array_load_int
+legalized:       true
+regBankSelected: false
+tracksRegLiveness: true
+body:             |
+  ; CHECK-LABEL: name: array_load_int
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
+  ; CHECK-NEXT:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @array_int
+  ; CHECK-NEXT:   %fp_load:gpr(s64) = G_LOAD [[GV]](p0) :: (dereferenceable load (s64) from @array_int)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[PHI:%[0-9]+]]:gpr(s32) = PHI %fp_load(s64), %bb.0, [[PHI]](s32), %bb.1
+  ; CHECK-NEXT:   G_BRCOND [[COPY]](s32), %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   $s0 = COPY [[PHI]](s32)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $s0
+  bb.0:
+    liveins: $w0
+    successors: %bb.1
+    %0:_(s32) = COPY $w0
+    %1:_(p0) = G_GLOBAL_VALUE @array_int
+    %fp_load:_(s64) = G_LOAD %1(p0) :: (dereferenceable load (s64) from @array_int)
+
+  bb.1:
+    successors: %bb.1, %bb.2
+    %2:_(s32) = PHI %fp_load, %bb.0, %2, %bb.1
+    G_BRCOND %0, %bb.1
+
+  bb.2:
+    $s0 = COPY %2
+    RET_ReallyLR implicit $s0
+...
+
+---
+name:            struct_array_load_int
+legalized:       true
+regBankSelected: false
+tracksRegLiveness: true
+body:             |
+  ; CHECK-LABEL: name: struct_array_load_int
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
+  ; CHECK-NEXT:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @struct_array_int
+  ; CHECK-NEXT:   %fp_load:gpr(s64) = G_LOAD [[GV]](p0) :: (dereferenceable load (s64) from @struct_array_int)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[PHI:%[0-9]+]]:gpr(s32) = PHI %fp_load(s64), %bb.0, [[PHI]](s32), %bb.1
+  ; CHECK-NEXT:   G_BRCOND [[COPY]](s32), %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   $s0 = COPY [[PHI]](s32)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $s0
+  bb.0:
+    liveins: $w0
+    successors: %bb.1
+    %0:_(s32) = COPY $w0
+    %1:_(p0) = G_GLOBAL_VALUE @struct_array_int
+    %fp_load:_(s64) = G_LOAD %1(p0) :: (dereferenceable load (s64) from @struct_array_int)
+
+  bb.1:
+    successors: %bb.1, %bb.2
+    %2:_(s32) = PHI %fp_load, %bb.0, %2, %bb.1
+    G_BRCOND %0, %bb.1
+
+  bb.2:
+    $s0 = COPY %2
+    RET_ReallyLR implicit $s0
+...
+
+---
+name:            struct_struct_array_load_int
+legalized:       true
+regBankSelected: false
+tracksRegLiveness: true
+body:             |
+  ; CHECK-LABEL: name: struct_struct_array_load_int
+  ; CHECK: bb.0:
+  ; CHECK-NEXT:   successors: %bb.1(0x80000000)
+  ; CHECK-NEXT:   liveins: $w0
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr(s32) = COPY $w0
+  ; CHECK-NEXT:   [[GV:%[0-9]+]]:gpr(p0) = G_GLOBAL_VALUE @struct_struct_array_int
+  ; CHECK-NEXT:   %fp_load:gpr(s64) = G_LOAD [[GV]](p0) :: (dereferenceable load (s64) from @struct_struct_array_int)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1:
+  ; CHECK-NEXT:   successors: %bb.1(0x40000000), %bb.2(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[PHI:%[0-9]+]]:gpr(s32) = PHI %fp_load(s64), %bb.0, [[PHI]](s32), %bb.1
+  ; CHECK-NEXT:   G_BRCOND [[COPY]](s32), %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2:
+  ; CHECK-NEXT:   $s0 = COPY [[PHI]](s32)
+  ; CHECK-NEXT:   RET_ReallyLR implicit $s0
+  bb.0:
+    liveins: $w0
+    successors: %bb.1
+    %0:_(s32) = COPY $w0
+    %1:_(p0) = G_GLOBAL_VALUE @struct_struct_array_int
+    %fp_load:_(s64) = G_LOAD %1(p0) :: (dereferenceable load (s64) from @struct_struct_array_int)
+
+  bb.1:
+    successors: %bb.1, %bb.2
+    %2:_(s32) = PHI %fp_load, %bb.0, %2, %bb.1
+    G_BRCOND %0, %bb.1
+
+  bb.2:
+    $s0 = COPY %2
+    RET_ReallyLR implicit $s0
+...

aemerson
aemerson reviewed Dec 11, 2023
View reviewed changes
Copy link
Contributor

@aemerson aemerson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you precommit the test checks as well?

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 12, 2023 •
edited 

:white_check_mark: With the latest revision this PR passed the C/C++ code formatter.

@chuongg3 chuongg3 force-pushed the GlobalISel_Detect_ArrayTypes_FPR branch 2 times, most recently from 6fef1a3 to 10c3d8b Compare December 13, 2023 09:58
davemgreen
davemgreen approved these changes Dec 13, 2023
View reviewed changes
Copy link
Collaborator

@davemgreen davemgreen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this LGTM. Nice fix.

llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp Outdated Show resolved Hide resolved
@chuongg3 chuongg3 force-pushed the GlobalISel_Detect_ArrayTypes_FPR branch 2 times, most recently from 9eeb51a to 2522355 Compare December 14, 2023 16:26
@chuongg3
[GlobalISel][AArch64] Look into array's element
948ae8a 
In AArch64RegisterBankInfo, IsFPOrFPType() does not work correctly
with ArrayTypes and StructTypes as it does not not look at their elements.

This caused some registers to be selected as gpr instead of fpr.
@chuongg3
fixup! [GlobalISel][AArch64] Look into array's element
9cfe460
@chuongg3
fixup! fixup! [GlobalISel][AArch64] Look into array's element
97df8e7
@chuongg3 chuongg3 force-pushed the GlobalISel_Detect_ArrayTypes_FPR branch from 2522355 to 97df8e7 Compare December 15, 2023 09:40
@chuongg3 chuongg3 merged commit 70579c9 into llvm:main Dec 15, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aemerson aemerson aemerson approved these changes

@davemgreen davemgreen davemgreen approved these changes

Assignees
No one assigned
Labels
backend:AArch64 llvm:globalisel
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@chuongg3 @llvmbot @aemerson @davemgreen