vtjnash added a comment.
Herald added a subscriber: StephenFan.
Herald added a project: All.
I was tracking back a recent ABI break (also failing now in gcc 12, so maybe
this irregularity is intentional), and was concerned that this commit is
observed to cause the platform ABI to change depending on the feature flags of
the current compilation unit. Prior to this change, f16 was always treated as
i16 for the purpose of the calling-convention (e.g. returned in %ax). But after
this change, the ABI of the value is now inconsistent between compile units. I
made a small change to one of the existing tests to show this. Note how the
callq result was in %ax without this mattr flag, and in %xmm0 with this mattr
flag added. But the function known as "identity.half" is external, and did not
change between those two calls to the llvm.
diff --git a/llvm/test/CodeGen/X86/half.ll b/llvm/test/CodeGen/X86/half.ll
index 46179e7d9113..8c1b8c4b76ff 100644
--- a/llvm/test/CodeGen/X86/half.ll
+++ b/llvm/test/CodeGen/X86/half.ll
@@ -5,6 +5,8 @@
; RUN: | FileCheck %s -check-prefixes=CHECK,CHECK-LIBCALL,BWOFF
; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mattr=+f16c
-fixup-byte-word-insts=1 \
; RUN: | FileCheck %s -check-prefixes=CHECK,BWON,BWON-F16C
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mattr=+avx512fp16
-fixup-byte-word-insts=0 \
+; RUN: | FileCheck %s -check-prefixes=CHECK-CC
; RUN: llc < %s -mtriple=i686-unknown-linux-gnu -mattr +sse2
-fixup-byte-word-insts=0 \
; RUN: | FileCheck %s -check-prefixes=CHECK-I686
@@ -163,16 +199,31 @@ define void @test_trunc32(float %in, half* %addr) #0 {
ret void
}
+declare half @identity.half(half)
+
define void @test_trunc64(double %in, half* %addr) #0 {
; CHECK-LABEL: test_trunc64:
; CHECK: # %bb.0:
; CHECK-NEXT: pushq %rbx
; CHECK-NEXT: movq %rdi, %rbx
; CHECK-NEXT: callq __truncdfhf2@PLT
+; CHECK-NEXT: # kill: def $ax killed $ax def $eax
+; CHECK-NEXT: movl %eax, %edi
+; CHECK-NEXT: callq identity.half@PLT
; CHECK-NEXT: movw %ax, (%rbx)
; CHECK-NEXT: popq %rbx
; CHECK-NEXT: retq
;
+; CHECK-CC-LABEL: test_trunc64:
+; CHECK-CC: # %bb.0:
+; CHECK-CC-NEXT: pushq %rbx
+; CHECK-CC-NEXT: movq %rdi, %rbx
+; CHECK-CC-NEXT: vcvtsd2sh %xmm0, %xmm0, %xmm0
+; CHECK-CC-NEXT: callq identity.half@PLT
+; CHECK-CC-NEXT: vmovsh %xmm0, (%rbx)
+; CHECK-CC-NEXT: popq %rbx
+; CHECK-CC-NEXT: retq
+;
; CHECK-I686-LABEL: test_trunc64:
; CHECK-I686: # %bb.0:
; CHECK-I686-NEXT: pushl %esi
@@ -181,12 +232,16 @@ define void @test_trunc64(double %in, half* %addr) #0 {
; CHECK-I686-NEXT: movsd {{.*#+}} xmm0 = mem[0],zero
; CHECK-I686-NEXT: movsd %xmm0, (%esp)
; CHECK-I686-NEXT: calll __truncdfhf2
+; CHECK-I686-NEXT: # kill: def $ax killed $ax def $eax
+; CHECK-I686-NEXT: movl %eax, (%esp)
+; CHECK-I686-NEXT: calll identity.half@PLT
; CHECK-I686-NEXT: movw %ax, (%esi)
; CHECK-I686-NEXT: addl $8, %esp
; CHECK-I686-NEXT: popl %esi
; CHECK-I686-NEXT: retl
%val16 = fptrunc double %in to half
- store half %val16, half* %addr
+ %val16b = call half @identity.half(half %val16)
+ store half %val16b, half* %addr
ret void
}
Is this intentional? We do already have code to handle the ABI dependency on
vector-sizes, and could add this to the list of flags that change the ABI (i.e.
we disable it if it will break the ABI), but wanted to confirm first if that
was the intent here.
discovered from https://github.com/JuliaLang/julia/issues/44829
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105263/new/
https://reviews.llvm.org/D105263
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
