[llvm] [AArch64][GlobalISel] Look into array's element (PR #74109)

via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 15 01:40:38 PST 2023


https://github.com/chuongg3 updated https://github.com/llvm/llvm-project/pull/74109

>From 948ae8a66dbb24b392cfe9bc8e9aac12e134b05e Mon Sep 17 00:00:00 2001
From: Tuan Chuong Goh <chuong.goh at arm.com>
Date: Tue, 28 Nov 2023 11:01:02 +0000
Subject: [PATCH 1/3] [GlobalISel][AArch64] Look into array's element

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.
---
 .../Target/AArch64/GISel/AArch64RegisterBankInfo.cpp | 11 +++++++++++
 .../AArch64/GlobalISel/regbankselect-fp-loads.mir    | 12 ++++++------
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
index 3284d0f033e3b4..263a9f1cec8736 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 a28056ddd0ac1f..6efa6f29d850af 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/regbankselect-fp-loads.mir
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/regbankselect-fp-loads.mir
@@ -123,12 +123,12 @@ body:             |
   ; 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:gpr(s64) = G_LOAD [[GV]](p0) :: (dereferenceable load (s64) from @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]+]]:gpr(s32) = PHI %fp_load(s64), %bb.0, [[PHI]](s32), %bb.1
+  ; 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:
@@ -164,12 +164,12 @@ body:             |
   ; 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:gpr(s64) = G_LOAD [[GV]](p0) :: (dereferenceable load (s64) from @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]+]]:gpr(s32) = PHI %fp_load(s64), %bb.0, [[PHI]](s32), %bb.1
+  ; 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:
@@ -205,12 +205,12 @@ body:             |
   ; 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:gpr(s64) = G_LOAD [[GV]](p0) :: (dereferenceable load (s64) from @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]+]]:gpr(s32) = PHI %fp_load(s64), %bb.0, [[PHI]](s32), %bb.1
+  ; 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:

>From 9cfe4603c7860f0e4605edd38901138c35fdf901 Mon Sep 17 00:00:00 2001
From: Tuan Chuong Goh <chuong.goh at arm.com>
Date: Tue, 12 Dec 2023 14:38:44 +0000
Subject: [PATCH 2/3] fixup! [GlobalISel][AArch64] Look into array's element

---
 llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
index 263a9f1cec8736..db6c8ea672a169 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
@@ -622,16 +622,12 @@ bool AArch64RegisterBankInfo::isLoadFromFPType(const MachineInstr &MI) const {
   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));
-      }
+    while (StructType *StructEltTy = dyn_cast<StructType>(EltTy)) {
       EltTy = StructEltTy->getTypeAtIndex(0U);
     }
     // Look at the first element of the array to determine its type
-    if (isa<ArrayType>(EltTy)) {
+    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.

>From 97df8e7dcf9550e93ec546c5c3046c8658e79af8 Mon Sep 17 00:00:00 2001
From: Tuan Chuong Goh <chuong.goh at arm.com>
Date: Thu, 14 Dec 2023 16:06:21 +0000
Subject: [PATCH 3/3] fixup! fixup! [GlobalISel][AArch64] Look into array's
 element

---
 llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
index db6c8ea672a169..b8e5e7bbdaba77 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
@@ -621,10 +621,10 @@ 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
-    while (StructType *StructEltTy = dyn_cast<StructType>(EltTy)) {
+    // Look at the first element of the struct to determine the type we are
+    // loading
+    while (StructType *StructEltTy = dyn_cast<StructType>(EltTy))
       EltTy = StructEltTy->getTypeAtIndex(0U);
-    }
     // Look at the first element of the array to determine its type
     if (isa<ArrayType>(EltTy))
       EltTy = EltTy->getArrayElementType();



More information about the llvm-commits mailing list