[PATCH] D136443: X86: use soft-float ABI for fp16 libcalls on Darwin

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:23236
+  if (VT == MVT::f16 && (SVT == MVT::f64 || SVT == MVT::f32) &&
+      !Subtarget.hasFP16() && (SVT == MVT::f64 || !Subtarget.hasF16C())) {
+    if (!Subtarget.getTargetTriple().isOSDarwin())
----------------
t.p.northover wrote:
> pengfei wrote:
> > I assume we have set it legal under FP16, why do we still check here?
> Apparently not. Assertions all over the place if I remove it.
Oh, yes, we need to handle f80/f128 here.


================
Comment at: llvm/test/CodeGen/X86/fp-round.ll:5
-; RUN: llc < %s -mtriple=x86_64-apple-darwin -mattr=+avx | FileCheck %s --check-prefixes=AVX1
-; RUN: llc < %s -mtriple=x86_64-apple-darwin -mattr=+avx512f,+avx512vl | FileCheck %s --check-prefixes=AVX512,AVX512F
-; RUN: llc < %s -mtriple=x86_64-apple-darwin -mattr=+avx512fp16,+avx512vl | FileCheck %s --check-prefixes=AVX512,AVX512FP16
----------------
t.p.northover wrote:
> pengfei wrote:
> > Why remove it? I think it helps reduce duplicated checks.
> The update tool didn't make use of it for whatever reason and printed a warning. Maybe the comments etc?
Probably.


================
Comment at: llvm/test/CodeGen/X86/fp-round.ll:596
+; AVX512FP16-LABEL: round_v16f32:
+; AVX512FP16:       ## %bb.0:
+; AVX512FP16-NEXT:    vpbroadcastd {{.*#+}} zmm1 = [4.9999997E-1,4.9999997E-1,4.9999997E-1,4.9999997E-1,4.9999997E-1,4.9999997E-1,4.9999997E-1,4.9999997E-1,4.9999997E-1,4.9999997E-1,4.9999997E-1,4.9999997E-1,4.9999997E-1,4.9999997E-1,4.9999997E-1,4.9999997E-1]
----------------
t.p.northover wrote:
> pengfei wrote:
> > One more `#`? Is it modified by manual? This should be updated by `utils/update_llc_test_checks.py` directly.
> This is the `update_llc_test_checks.py` output. `CommentString` is "##" on Darwin for apparently historical reasons:
> 
> ```
>   // Use ## as a comment string so that .s files generated by llvm can go
>   // through the GCC preprocessor without causing an error.  This is needed
>   // because "clang foo.s" runs the C preprocessor, which is usually reserved
>   // for .S files on other systems.  Perhaps this is because the file system
>   // wasn't always case preserving or something.
> ```
> 
> Long before my time.
OK, I see the problem now. Others are not for Darwin.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136443/new/

https://reviews.llvm.org/D136443