This is an archive of the discontinued LLVM Phabricator instance.

X86: use soft-float ABI for fp16 libcalls on Darwin
ClosedPublic

Authored by t.p.northover on Oct 21 2022, 5:40 AM.

Details

Reviewers
pengfei
Summary

We've been shipping implementations of fp16 libcalls in MacOS since 10.10 in 2014 I believe, and there are apps around using them so for the libcall emulation we need to stick with the old ABI that uses GPRs. We think normal function calls should be unaffected.

Fortunately it's not that big of a problem since performance is going to be terrible anyway, just a bit of an annoyance in ISelLowering. The alternative implementation I considered was adding a new calling convention and using setLibcallCallingConv, less code (and more obviously correct if-conditions) but puts gory details into include. I'd be happy to switch if people think I went the wrong way.

There's a bit of test churn as I switched the generic failures over to using a triple with the common case rather than this exceptional behaviour, and that changed some of the names and exact comment format.

Diff Detail

Event Timeline

t.p.northover created this revision.Oct 21 2022, 5:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 21 2022, 5:40 AM
t.p.northover requested review of this revision.Oct 21 2022, 5:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 21 2022, 5:40 AM
pengfei added inline comments.Oct 21 2022, 5:56 AM
llvm/test/CodeGen/X86/fp-round.ll
5

Why remove it? I think it helps reduce duplicated checks.

596

One more #? Is it modified by manual? This should be updated by utils/update_llc_test_checks.py directly.

In fact I noticed this problem when switching the ABI and tried to fix it in the compiler-rt side, see D128872.

pengfei added inline comments.Oct 21 2022, 6:05 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
23236

I assume we have set it legal under FP16, why do we still check here?

t.p.northover added inline comments.Oct 25 2022, 3:51 AM
llvm/test/CodeGen/X86/fp-round.ll
5

The update tool didn't make use of it for whatever reason and printed a warning. Maybe the comments etc?

596

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.

In fact I noticed this problem when switching the ABI and tried to fix it in the compiler-rt side, see D128872.

Ah, interesting. I started out the week with that (or something like it) as my preferred option too. But then I had a grep through my /Applications and found something that would break in just my limited sample (Hex Fiend uses them from libcompiler_rt.dylib, which has been shipping for ages), so decided I there's almost certainly lots more out there I don't know about and couldn't make that change.

llvm/lib/Target/X86/X86ISelLowering.cpp
23236

Apparently not. Assertions all over the place if I remove it.

pengfei added inline comments.Oct 25 2022, 4:50 AM
llvm/lib/Target/X86/X86ISelLowering.cpp
23236

Oh, yes, we need to handle f80/f128 here.

llvm/test/CodeGen/X86/fp-round.ll
5

Probably.

596

OK, I see the problem now. Others are not for Darwin.

pengfei accepted this revision.Nov 8 2022, 6:52 AM

I'm not familiar with the darwin target. But I can see it's NFC for non-darwin targets, which LGTM.

This revision is now accepted and ready to land.Nov 8 2022, 6:52 AM
t.p.northover closed this revision.Nov 10 2022, 2:00 AM

Thanks. Committed as 2bcf51c7f82c.