[Arm64EC] Add support for fp128 - #206980
Conversation
|
cc @tgross35 I'm not entirely sure how to unwind the big stack of issues here, but maybe we can merge this first despite the lack of test coverage? |
|
@llvm/pr-subscribers-backend-aarch64 Author: Folkert de Vries (folkertdev) Changesfixes #94434 Analogue to #152843 but for Testing is a bit thin still, we need to fix some other problems like #144006 so that the arm64ec target can be added to more tests. Full diff: https://github.com/llvm/llvm-project/pull/206980.diff 5 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp b/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
index 44f76f9e8772b..147fa490c32a3 100644
--- a/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
@@ -333,9 +333,16 @@ ThunkArgInfo AArch64Arm64ECCallLowering::canonicalizeThunkType(
return direct(T);
}
+ if (T->isFP128Ty()) {
+ // Prefix with `llvm` since MSVC doesn't specify `_Float128`
+ Out << "__llvm_q__";
+ // f128 uses sret for compatibility with GCC.
+ return pointerIndirection(T);
+ }
+
if (T->isFloatingPointTy()) {
- report_fatal_error("Only 16, 32, and 64 bit floating points are supported "
- "for ARM64EC thunks");
+ report_fatal_error("Only 16, 32, 64, and 128 bit IEEE floating points "
+ "are supported for ARM64EC thunks");
}
auto &DL = M->getDataLayout();
@@ -350,7 +357,7 @@ ThunkArgInfo AArch64Arm64ECCallLowering::canonicalizeThunkType(
uint64_t ElementSizePerBytes = DL.getTypeSizeInBits(ElementTy) / 8;
uint64_t TotalSizeBytes = ElementCnt * ElementSizePerBytes;
if (ElementTy->isHalfTy() || ElementTy->isFloatTy() ||
- ElementTy->isDoubleTy()) {
+ ElementTy->isDoubleTy() || ElementTy->isFP128Ty()) {
if (ElementTy->isHalfTy())
// Prefix with `llvm` since MSVC doesn't specify `_Float16`
Out << "__llvm_H__";
@@ -358,6 +365,9 @@ ThunkArgInfo AArch64Arm64ECCallLowering::canonicalizeThunkType(
Out << "F";
else if (ElementTy->isDoubleTy())
Out << "D";
+ else if (ElementTy->isFP128Ty())
+ // Prefix with `llvm` since MSVC doesn't specify `_Float128`
+ Out << "__llvm_Q__";
Out << TotalSizeBytes;
if (Alignment.value() >= 16 && !Ret)
Out << "a" << Alignment.value();
@@ -370,9 +380,8 @@ ThunkArgInfo AArch64Arm64ECCallLowering::canonicalizeThunkType(
return pointerIndirection(T);
}
} else if (T->isFloatingPointTy()) {
- report_fatal_error(
- "Only 16, 32, and 64 bit floating points are supported "
- "for ARM64EC thunks");
+ report_fatal_error("Only 16, 32, 64, and 128 bit IEEE floating points "
+ "are supported for ARM64EC thunks");
}
}
diff --git a/llvm/test/CodeGen/AArch64/arm64ec-entry-thunks.ll b/llvm/test/CodeGen/AArch64/arm64ec-entry-thunks.ll
index 09fe884940c6a..e1ffd7655c232 100644
--- a/llvm/test/CodeGen/AArch64/arm64ec-entry-thunks.ll
+++ b/llvm/test/CodeGen/AArch64/arm64ec-entry-thunks.ll
@@ -85,10 +85,10 @@ define i64 @simple_integers(i8, i16, i32, i64) nounwind {
ret i64 0
}
-; NOTE: Only half, float, and double are supported.
-define double @simple_floats(half, float, double) nounwind {
-; CHECK-LABEL: .def $ientry_thunk$cdecl$d$__llvm_h__fd;
-; CHECK: .section .wowthk$aa,"xr",discard,$ientry_thunk$cdecl$d$__llvm_h__fd
+; NOTE: Only half, float, double, and fp128 are supported.
+define double @simple_floats(half, float, double, fp128) nounwind {
+; CHECK-LABEL: .def $ientry_thunk$cdecl$d$__llvm_h__fd__llvm_q__;
+; CHECK: .section .wowthk$aa,"xr",discard,$ientry_thunk$cdecl$d$__llvm_h__fd__llvm_q__
; CHECK: // %bb.0:
; CHECK-NEXT: stp q6, q7, [sp, #-176]! // 32-byte Folded Spill
; CHECK-NEXT: .seh_save_any_reg_px q6, 176
@@ -105,6 +105,7 @@ define double @simple_floats(half, float, double) nounwind {
; CHECK-NEXT: add x29, sp, #160
; CHECK-NEXT: .seh_add_fp 160
; CHECK-NEXT: .seh_endprologue
+; CHECK-NEXT: ldr q3, [x3]
; CHECK-NEXT: blr x9
; CHECK-NEXT: adrp x8, __os_arm64x_dispatch_ret
; CHECK-NEXT: ldr x0, [x8, :lo12:__os_arm64x_dispatch_ret]
@@ -602,7 +603,7 @@ start:
; CHECK-NEXT: .symidx $ientry_thunk$cdecl$i8$i8i8i8i8
; CHECK-NEXT: .word 1
; CHECK-NEXT: .symidx "#simple_floats"
-; CHECK-NEXT: .symidx $ientry_thunk$cdecl$d$__llvm_h__fd
+; CHECK-NEXT: .symidx $ientry_thunk$cdecl$d$__llvm_h__fd__llvm_q__
; CHECK-NEXT: .word 1
; CHECK-NEXT: .symidx "#has_varargs"
; CHECK-NEXT: .symidx $ientry_thunk$cdecl$v$varargs
diff --git a/llvm/test/CodeGen/AArch64/arm64ec-exit-thunks.ll b/llvm/test/CodeGen/AArch64/arm64ec-exit-thunks.ll
index 6fba6a3974574..22ca2bb59fd87 100644
--- a/llvm/test/CodeGen/AArch64/arm64ec-exit-thunks.ll
+++ b/llvm/test/CodeGen/AArch64/arm64ec-exit-thunks.ll
@@ -93,26 +93,28 @@ declare i64 @simple_integers(i8, i16, i32, i64) nounwind;
; CHECK-NEXT: .seh_endfunclet
; CHECK-NEXT: .seh_endproc
-; NOTE: Only half, float, and double are supported.
-declare double @simple_floats(half, float, double) nounwind;
-; CHECK-LABEL: .def $iexit_thunk$cdecl$d$__llvm_h__fd;
-; CHECK: .section .wowthk$aa,"xr",discard,$iexit_thunk$cdecl$d$__llvm_h__fd
+; NOTE: Only half, float, double, and fp128 are supported.
+declare double @simple_floats(half, float, double, fp128) nounwind;
+; CHECK-LABEL: .def $iexit_thunk$cdecl$d$__llvm_h__fd__llvm_q__;
+; CHECK: .section .wowthk$aa,"xr",discard,$iexit_thunk$cdecl$d$__llvm_h__fd__llvm_q__
; CHECK: // %bb.0:
-; CHECK-NEXT: sub sp, sp, #48
-; CHECK-NEXT: .seh_stackalloc 48
-; CHECK-NEXT: stp x29, x30, [sp, #32] // 16-byte Folded Spill
-; CHECK-NEXT: .seh_save_fplr 32
-; CHECK-NEXT: add x29, sp, #32
-; CHECK-NEXT: .seh_add_fp 32
+; CHECK-NEXT: sub sp, sp, #64
+; CHECK-NEXT: .seh_stackalloc 64
+; CHECK-NEXT: stp x29, x30, [sp, #48] // 16-byte Folded Spill
+; CHECK-NEXT: .seh_save_fplr 48
+; CHECK-NEXT: add x29, sp, #48
+; CHECK-NEXT: .seh_add_fp 48
; CHECK-NEXT: .seh_endprologue
; CHECK-NEXT: adrp x8, __os_arm64x_dispatch_call_no_redirect
+; CHECK-NEXT: sub x3, x29, #16
+; CHECK-NEXT: stur q3, [x29, #-16]
; CHECK-NEXT: ldr x16, [x8, :lo12:__os_arm64x_dispatch_call_no_redirect]
; CHECK-NEXT: blr x16
; CHECK-NEXT: .seh_startepilogue
-; CHECK-NEXT: ldp x29, x30, [sp, #32] // 16-byte Folded Reload
-; CHECK-NEXT: .seh_save_fplr 32
-; CHECK-NEXT: add sp, sp, #48
-; CHECK-NEXT: .seh_stackalloc 48
+; CHECK-NEXT: ldp x29, x30, [sp, #48] // 16-byte Folded Reload
+; CHECK-NEXT: .seh_save_fplr 48
+; CHECK-NEXT: add sp, sp, #64
+; CHECK-NEXT: .seh_stackalloc 64
; CHECK-NEXT: .seh_endepilogue
; CHECK-NEXT: ret
; CHECK-NEXT: .seh_endfunclet
@@ -129,8 +131,8 @@ declare double @simple_floats(half, float, double) nounwind;
; CHECK-NEXT: adrp x11, simple_floats
; CHECK-NEXT: add x11, x11, :lo12:simple_floats
; CHECK-NEXT: ldr x8, [x8, :lo12:__os_arm64x_check_icall]
-; CHECK-NEXT: adrp x10, $iexit_thunk$cdecl$d$__llvm_h__fd
-; CHECK-NEXT: add x10, x10, :lo12:$iexit_thunk$cdecl$d$__llvm_h__fd
+; CHECK-NEXT: adrp x10, $iexit_thunk$cdecl$d$__llvm_h__fd__llvm_q__
+; CHECK-NEXT: add x10, x10, :lo12:$iexit_thunk$cdecl$d$__llvm_h__fd__llvm_q__
; CHECK-NEXT: blr x8
; CHECK-NEXT: .seh_startepilogue
; CHECK-NEXT: ldr x30, [sp], #16 // 8-byte Folded Reload
@@ -615,7 +617,7 @@ declare void @"??@md5mangleaaaaaaaaaaaaaaaaaaaaaaa@"()
; CHECK-NEXT: .symidx simple_integers
; CHECK-NEXT: .word 0
; CHECK-NEXT: .symidx simple_floats
-; CHECK-NEXT: .symidx $iexit_thunk$cdecl$d$__llvm_h__fd
+; CHECK-NEXT: .symidx $iexit_thunk$cdecl$d$__llvm_h__fd__llvm_q__
; CHECK-NEXT: .word 4
; CHECK-NEXT: .symidx "#simple_floats$exit_thunk"
; CHECK-NEXT: .symidx simple_floats
@@ -678,7 +680,7 @@ declare void @"??@md5mangleaaaaaaaaaaaaaaaaaaaaaaa@"()
define void @func_caller() nounwind {
call void @no_op()
call i64 @simple_integers(i8 0, i16 0, i32 0, i64 0)
- call double @simple_floats(half 0.0, float 0.0, double 0.0)
+ call double @simple_floats(half 0.0, float 0.0, double 0.0, fp128 0.0)
call void (...) @has_varargs()
%c = alloca i8
call void @has_sret(ptr sret([100 x i8]) %c)
diff --git a/llvm/test/CodeGen/Generic/fp128-exp10-libcall.ll b/llvm/test/CodeGen/Generic/fp128-exp10-libcall.ll
index 5e97f03c2bc3b..50956b5e93f38 100644
--- a/llvm/test/CodeGen/Generic/fp128-exp10-libcall.ll
+++ b/llvm/test/CodeGen/Generic/fp128-exp10-libcall.ll
@@ -13,7 +13,10 @@
; RUN: %if x86-registered-target %{ llc < %s -mtriple=i686-unknown-linux-musl | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-USELD %}
; RUN: %if x86-registered-target %{ llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-F128 %}
; RUN: %if x86-registered-target %{ llc < %s -mtriple=x86_64-unknown-linux-musl | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-USELD %}
-; RUN %if x86-registered-target %{ llc < %s -mtriple=x86_64-pc-windows-msvc | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-F128 %}
+
+; FIXME(#144006): Windows-MSVC should also be run but has a ldexp selection failure.
+; %if x86-registered-target %{ llc < %s -mtriple=x86_64-pc-windows-msvc | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-F128 %}
+; %if aarch64-registered-target %{ llc < %s -mtriple=arm64ec-pc-windows-msvc | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-F128 %}
; ERR: error: no libcall available for fexp10
define fp128 @test_exp10(fp128 %a) {
diff --git a/llvm/test/CodeGen/Generic/fp128-math-libcalls.ll b/llvm/test/CodeGen/Generic/fp128-math-libcalls.ll
index f759c94621381..ed8b9b0789c2e 100644
--- a/llvm/test/CodeGen/Generic/fp128-math-libcalls.ll
+++ b/llvm/test/CodeGen/Generic/fp128-math-libcalls.ll
@@ -30,10 +30,10 @@
; RUN: %if x86-registered-target %{ llc < %s -mtriple=i686-unknown-linux-musl | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-USELD %}
; RUN: %if x86-registered-target %{ llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-F128 %}
; RUN: %if x86-registered-target %{ llc < %s -mtriple=x86_64-unknown-linux-musl | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-USELD %}
-;
-; FIXME(#144006): Windows-MSVC should also be run but has a ldexp selection
-; failure.
-; %if x86-registered-target %{ llc < %s -mtriple=x86_64-pc-windows-msvc -verify-machineinstrs | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-F128 %}
+
+; FIXME(#144006): Windows-MSVC should also be run but has a ldexp selection failure.
+; %if x86-registered-target %{ llc < %s -mtriple=x86_64-pc-windows-msvc | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-F128 %}
+; %if aarch64-registered-target %{ llc < %s -mtriple=arm64ec-pc-windows-msvc | FileCheck %s --check-prefixes=CHECK-ALL,CHECK-F128 %}
define fp128 @test_acos(fp128 %a) {
; CHECK-ALL-LABEL: test_acos:
|
efriedma-quic
left a comment
There was a problem hiding this comment.
I'm okay with landing the bits we can, even if the test coverage isn't as complete as we'd like.
| Out << "D"; | ||
| else if (ElementTy->isFP128Ty()) | ||
| // Prefix with `llvm` since MSVC doesn't specify `_Float128` | ||
| Out << "__llvm_Q__"; |
There was a problem hiding this comment.
Missing test coverage.
| // f128 uses sret for compatibility with GCC. | ||
| return pointerIndirection(T); |
There was a problem hiding this comment.
This also handles the pass ABI right?
There was a problem hiding this comment.
I think so, you mean the comment should also mention that?
There was a problem hiding this comment.
Mostly just for my clarification, but the comment doesn't hurt
efriedma-quic
left a comment
There was a problem hiding this comment.
I think I'd also like to see a testcase for an f128 return value, to show the register->sret translation works correctly.
Otherwise LGTM
arm64ec: `f128` is supported since LLVM 23 tracking issue: rust-lang#116909 - [x] I did not use an LLM to create a change in this PR. - [ ] I used an LLM to create a change in this PR, and I have explained below how it was used. Related - llvm/llvm-project#94434 - llvm/llvm-project#206980 cc @dpaoliello (feel free to approve also, and maybe you can validate this in practice?) r? tgross35
arm64ec: `f128` is supported since LLVM 23 tracking issue: rust-lang#116909 - [x] I did not use an LLM to create a change in this PR. - [ ] I used an LLM to create a change in this PR, and I have explained below how it was used. Related - llvm/llvm-project#94434 - llvm/llvm-project#206980 cc @dpaoliello (feel free to approve also, and maybe you can validate this in practice?) r? tgross35
arm64ec: `f128` is supported since LLVM 23 tracking issue: rust-lang#116909 - [x] I did not use an LLM to create a change in this PR. - [ ] I used an LLM to create a change in this PR, and I have explained below how it was used. Related - llvm/llvm-project#94434 - llvm/llvm-project#206980 cc @dpaoliello (feel free to approve also, and maybe you can validate this in practice?) r? tgross35
Rollup merge of #160779 - folkertdev:arm64ec-f128, r=tgross35 arm64ec: `f128` is supported since LLVM 23 tracking issue: #116909 - [x] I did not use an LLM to create a change in this PR. - [ ] I used an LLM to create a change in this PR, and I have explained below how it was used. Related - llvm/llvm-project#94434 - llvm/llvm-project#206980 cc @dpaoliello (feel free to approve also, and maybe you can validate this in practice?) r? tgross35
fixes #94434
Analogue to #152843 but for
fp128.Testing is a bit thin still, we need to fix some other problems like #144006 so that the arm64ec target can be added to more tests.