[llvm] [AArch64][GlobalISel] Fix incorrect codegen for FPR16/FPR8 to GPR copies (PR #171499)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 9 12:38:11 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-aarch64
Author: Ian Butterworth (IanButterworth)
<details>
<summary>Changes</summary>
I reported https://github.com/llvm/llvm-project/issues/171494 and thought I would see if Claude (opus 4.5) was able to suggest a fix.
This PR is the product of that.
Please note that I am not familiar with LLVM development, and this should be reviewed with that in mind.
Claude-generated explanation:
----
Previously, copyPhysReg() was missing handlers for copies between FPR16/FPR8 and GPR32/GPR64 register classes. These cases fell through to the NZCV handler, which incorrectly generated 'mrs Rd, NZCV' instead of the proper FMOV instruction.
This caused incorrect code generation for patterns like:
%ival = bitcast half %val to i16
store atomic i16 %ival, ptr %addr release, align 2
Which generated 'mrs w8, NZCV' instead of 'fmov w8, h0'.
The fix adds proper copy handlers:
- FPR16 <-> GPR32: Use FMOVHWr/FMOVWHr with FullFP16, otherwise promote to FPR32 super-register and use FMOVSWr/FMOVWSr
- FPR16 <-> GPR64: Use FMOVHXr/FMOVXHr with FullFP16, otherwise promote to FPR64 super-register and use FMOVDXr/FMOVXDr
- FPR8 <-> GPR32: Promote to FPR32 and use FMOVSWr/FMOVWSr
- FPR8 <-> GPR64: Promote to FPR64 and use FMOVDXr/FMOVXDr
Fixes https://github.com/llvm/llvm-project/issues/171494
---
Full diff: https://github.com/llvm/llvm-project/pull/171499.diff
2 Files Affected:
- (modified) llvm/lib/Target/AArch64/AArch64InstrInfo.cpp (+96)
- (added) llvm/test/CodeGen/AArch64/GlobalISel/arm64-atomic-store-fp16.ll (+109)
``````````diff
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index f82180fc57b99..5027acec851cf 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -5851,6 +5851,102 @@ void AArch64InstrInfo::copyPhysReg(MachineBasicBlock &MBB,
return;
}
+ // Copies between GPR32 and FPR16.
+ if (AArch64::FPR16RegClass.contains(DestReg) &&
+ AArch64::GPR32RegClass.contains(SrcReg)) {
+ if (Subtarget.hasFullFP16()) {
+ BuildMI(MBB, I, DL, get(AArch64::FMOVWHr), DestReg)
+ .addReg(SrcReg, getKillRegState(KillSrc));
+ } else {
+ MCRegister DestRegS =
+ RI.getMatchingSuperReg(DestReg, AArch64::hsub, &AArch64::FPR32RegClass);
+ BuildMI(MBB, I, DL, get(AArch64::FMOVWSr), DestRegS)
+ .addReg(SrcReg, getKillRegState(KillSrc));
+ }
+ return;
+ }
+ if (AArch64::GPR32RegClass.contains(DestReg) &&
+ AArch64::FPR16RegClass.contains(SrcReg)) {
+ if (Subtarget.hasFullFP16()) {
+ BuildMI(MBB, I, DL, get(AArch64::FMOVHWr), DestReg)
+ .addReg(SrcReg, getKillRegState(KillSrc));
+ } else {
+ MCRegister SrcRegS =
+ RI.getMatchingSuperReg(SrcReg, AArch64::hsub, &AArch64::FPR32RegClass);
+ BuildMI(MBB, I, DL, get(AArch64::FMOVSWr), DestReg)
+ .addReg(SrcRegS, RegState::Undef)
+ .addReg(SrcReg, RegState::Implicit | getKillRegState(KillSrc));
+ }
+ return;
+ }
+
+ // Copies between GPR64 and FPR16.
+ if (AArch64::FPR16RegClass.contains(DestReg) &&
+ AArch64::GPR64RegClass.contains(SrcReg)) {
+ if (Subtarget.hasFullFP16()) {
+ BuildMI(MBB, I, DL, get(AArch64::FMOVXHr), DestReg)
+ .addReg(SrcReg, getKillRegState(KillSrc));
+ } else {
+ MCRegister DestRegD =
+ RI.getMatchingSuperReg(DestReg, AArch64::hsub, &AArch64::FPR64RegClass);
+ BuildMI(MBB, I, DL, get(AArch64::FMOVXDr), DestRegD)
+ .addReg(SrcReg, getKillRegState(KillSrc));
+ }
+ return;
+ }
+ if (AArch64::GPR64RegClass.contains(DestReg) &&
+ AArch64::FPR16RegClass.contains(SrcReg)) {
+ if (Subtarget.hasFullFP16()) {
+ BuildMI(MBB, I, DL, get(AArch64::FMOVHXr), DestReg)
+ .addReg(SrcReg, getKillRegState(KillSrc));
+ } else {
+ MCRegister SrcRegD =
+ RI.getMatchingSuperReg(SrcReg, AArch64::hsub, &AArch64::FPR64RegClass);
+ BuildMI(MBB, I, DL, get(AArch64::FMOVDXr), DestReg)
+ .addReg(SrcRegD, RegState::Undef)
+ .addReg(SrcReg, RegState::Implicit | getKillRegState(KillSrc));
+ }
+ return;
+ }
+
+ // Copies between GPR32 and FPR8.
+ if (AArch64::FPR8RegClass.contains(DestReg) &&
+ AArch64::GPR32RegClass.contains(SrcReg)) {
+ MCRegister DestRegS =
+ RI.getMatchingSuperReg(DestReg, AArch64::bsub, &AArch64::FPR32RegClass);
+ BuildMI(MBB, I, DL, get(AArch64::FMOVWSr), DestRegS)
+ .addReg(SrcReg, getKillRegState(KillSrc));
+ return;
+ }
+ if (AArch64::GPR32RegClass.contains(DestReg) &&
+ AArch64::FPR8RegClass.contains(SrcReg)) {
+ MCRegister SrcRegS =
+ RI.getMatchingSuperReg(SrcReg, AArch64::bsub, &AArch64::FPR32RegClass);
+ BuildMI(MBB, I, DL, get(AArch64::FMOVSWr), DestReg)
+ .addReg(SrcRegS, RegState::Undef)
+ .addReg(SrcReg, RegState::Implicit | getKillRegState(KillSrc));
+ return;
+ }
+
+ // Copies between GPR64 and FPR8.
+ if (AArch64::FPR8RegClass.contains(DestReg) &&
+ AArch64::GPR64RegClass.contains(SrcReg)) {
+ MCRegister DestRegD =
+ RI.getMatchingSuperReg(DestReg, AArch64::bsub, &AArch64::FPR64RegClass);
+ BuildMI(MBB, I, DL, get(AArch64::FMOVXDr), DestRegD)
+ .addReg(SrcReg, getKillRegState(KillSrc));
+ return;
+ }
+ if (AArch64::GPR64RegClass.contains(DestReg) &&
+ AArch64::FPR8RegClass.contains(SrcReg)) {
+ MCRegister SrcRegD =
+ RI.getMatchingSuperReg(SrcReg, AArch64::bsub, &AArch64::FPR64RegClass);
+ BuildMI(MBB, I, DL, get(AArch64::FMOVDXr), DestReg)
+ .addReg(SrcRegD, RegState::Undef)
+ .addReg(SrcReg, RegState::Implicit | getKillRegState(KillSrc));
+ return;
+ }
+
if (DestReg == AArch64::NZCV) {
assert(AArch64::GPR64RegClass.contains(SrcReg) && "Invalid NZCV copy");
BuildMI(MBB, I, DL, get(AArch64::MSR))
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/arm64-atomic-store-fp16.ll b/llvm/test/CodeGen/AArch64/GlobalISel/arm64-atomic-store-fp16.ll
new file mode 100644
index 0000000000000..0109b4d7c4c08
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/arm64-atomic-store-fp16.ll
@@ -0,0 +1,109 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=arm64-apple-ios -global-isel -global-isel-abort=1 -verify-machineinstrs | FileCheck %s --check-prefix=CHECK-NOFP16
+; RUN: llc < %s -mtriple=arm64-apple-ios -mattr=+fullfp16 -global-isel -global-isel-abort=1 -verify-machineinstrs | FileCheck %s --check-prefix=CHECK-FP16
+
+; Test for https://github.com/llvm/llvm-project/issues/171494
+; Atomic store of bitcast half to i16 was generating incorrect code (mrs instead of fmov).
+
+define void @atomic_store_half(ptr %addr, half %val) {
+; CHECK-NOFP16-LABEL: atomic_store_half:
+; CHECK-NOFP16: ; %bb.0:
+; CHECK-NOFP16-NEXT: fmov w8, s0
+; CHECK-NOFP16-NEXT: stlrh w8, [x0]
+; CHECK-NOFP16-NEXT: ret
+;
+; CHECK-FP16-LABEL: atomic_store_half:
+; CHECK-FP16: ; %bb.0:
+; CHECK-FP16-NEXT: fmov w8, h0
+; CHECK-FP16-NEXT: stlrh w8, [x0]
+; CHECK-FP16-NEXT: ret
+ %ival = bitcast half %val to i16
+ store atomic i16 %ival, ptr %addr release, align 2
+ ret void
+}
+
+define half @atomic_load_half(ptr %addr) {
+; CHECK-NOFP16-LABEL: atomic_load_half:
+; CHECK-NOFP16: ; %bb.0:
+; CHECK-NOFP16-NEXT: ldarh w8, [x0]
+; CHECK-NOFP16-NEXT: fmov s0, w8
+; CHECK-NOFP16-NEXT: ret
+;
+; CHECK-FP16-LABEL: atomic_load_half:
+; CHECK-FP16: ; %bb.0:
+; CHECK-FP16-NEXT: ldarh w8, [x0]
+; CHECK-FP16-NEXT: fmov h0, w8
+; CHECK-FP16-NEXT: ret
+ %ival = load atomic i16, ptr %addr acquire, align 2
+ %val = bitcast i16 %ival to half
+ ret half %val
+}
+
+define void @atomic_store_bfloat(ptr %addr, bfloat %val) {
+; CHECK-NOFP16-LABEL: atomic_store_bfloat:
+; CHECK-NOFP16: ; %bb.0:
+; CHECK-NOFP16-NEXT: fmov w8, s0
+; CHECK-NOFP16-NEXT: stlrh w8, [x0]
+; CHECK-NOFP16-NEXT: ret
+;
+; CHECK-FP16-LABEL: atomic_store_bfloat:
+; CHECK-FP16: ; %bb.0:
+; CHECK-FP16-NEXT: fmov w8, h0
+; CHECK-FP16-NEXT: stlrh w8, [x0]
+; CHECK-FP16-NEXT: ret
+ %ival = bitcast bfloat %val to i16
+ store atomic i16 %ival, ptr %addr release, align 2
+ ret void
+}
+
+define bfloat @atomic_load_bfloat(ptr %addr) {
+; CHECK-NOFP16-LABEL: atomic_load_bfloat:
+; CHECK-NOFP16: ; %bb.0:
+; CHECK-NOFP16-NEXT: ldarh w8, [x0]
+; CHECK-NOFP16-NEXT: fmov s0, w8
+; CHECK-NOFP16-NEXT: ret
+;
+; CHECK-FP16-LABEL: atomic_load_bfloat:
+; CHECK-FP16: ; %bb.0:
+; CHECK-FP16-NEXT: ldarh w8, [x0]
+; CHECK-FP16-NEXT: fmov h0, w8
+; CHECK-FP16-NEXT: ret
+ %ival = load atomic i16, ptr %addr acquire, align 2
+ %val = bitcast i16 %ival to bfloat
+ ret bfloat %val
+}
+
+; Test FPR8 to GPR32 copies (bitcast <1 x i8> to i8 for atomic store)
+define void @atomic_store_v1i8(ptr %addr, <1 x i8> %val) {
+; CHECK-NOFP16-LABEL: atomic_store_v1i8:
+; CHECK-NOFP16: ; %bb.0:
+; CHECK-NOFP16-NEXT: fmov w8, s0
+; CHECK-NOFP16-NEXT: stlrb w8, [x0]
+; CHECK-NOFP16-NEXT: ret
+;
+; CHECK-FP16-LABEL: atomic_store_v1i8:
+; CHECK-FP16: ; %bb.0:
+; CHECK-FP16-NEXT: fmov w8, s0
+; CHECK-FP16-NEXT: stlrb w8, [x0]
+; CHECK-FP16-NEXT: ret
+ %ival = bitcast <1 x i8> %val to i8
+ store atomic i8 %ival, ptr %addr release, align 1
+ ret void
+}
+
+define <1 x i8> @atomic_load_v1i8(ptr %addr) {
+; CHECK-NOFP16-LABEL: atomic_load_v1i8:
+; CHECK-NOFP16: ; %bb.0:
+; CHECK-NOFP16-NEXT: ldarb w8, [x0]
+; CHECK-NOFP16-NEXT: fmov s0, w8
+; CHECK-NOFP16-NEXT: ret
+;
+; CHECK-FP16-LABEL: atomic_load_v1i8:
+; CHECK-FP16: ; %bb.0:
+; CHECK-FP16-NEXT: ldarb w8, [x0]
+; CHECK-FP16-NEXT: fmov s0, w8
+; CHECK-FP16-NEXT: ret
+ %ival = load atomic i8, ptr %addr acquire, align 1
+ %val = bitcast i8 %ival to <1 x i8>
+ ret <1 x i8> %val
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/171499
More information about the llvm-commits
mailing list