pengfei added inline comments.
================
Comment at: clang/lib/Basic/Targets/X86.cpp:385
- if (!HasX87) {
- if (LongDoubleFormat == &llvm::APFloat::x87DoubleExtended())
- HasLongDouble = false;
- if (getTriple().getArch() == llvm::Triple::x86)
- HasFPReturn = false;
- }
+ if (!HasX87 && getTriple().getArch() == llvm::Triple::x86_64 &&
+ LongDoubleFormat == &llvm::APFloat::x87DoubleExtended())
----------------
asavonic wrote:
> pengfei wrote:
> > asavonic wrote:
> > > I see that D112143 changed the ABI so that FP return values do not use
> > > x87 registers on i386. Therefore HasFPReturn flag can be removed.
> > >
> > > However, operations with long double (x87 80-bit) should still be
> > > unsupported on both targets, because IIRC there is no SSE equivalent for
> > > them. GCC compiles them as soft-fp when -mno-x87 is set, but I haven't
> > > found 80-bit soft-fp implementation in LLVM.
> > > ```
> > > long double baz(long double a, long double b) {
> > > return a + b;
> > > }
> > > ```
> > >
> > > ```
> > > baz:
> > > [...]
> > > call __addxf3
> > > ```
> > > For some reason GCC only does this for for i386 target, for x86_64 it
> > > just emits the diagnostic about disabled x87.
> > Thanks for looking at this patch.
> > I don't think we need to exclude f80 particularly. IIUC, backend tries all
> > possible ways to lower a given operation. Lowering to library is always the
> > last choice. So the behavior is not confined to soft-fp.
> > It's true LLVM has problems with f80 lowering without x87. I commented it
> > in D112143 and hope D100091 will fix them. We don't need to bother to
> > change it again in future.
> >
> > > For some reason GCC only does this for for i386 target, for x86_64 it
> > > just emits the diagnostic about disabled x87.
> > I think the root reason is the difference in ABI. 32-bits ABI allows
> > passing and returning f80 without x87 registers while 64-bits doesn't. So
> > we have to and only need to disable it for x86_64.
> > I don't think we need to exclude f80 particularly. IIUC, backend tries all
> > possible ways to lower a given operation. Lowering to library is always the
> > last choice. So the behavior is not confined to soft-fp.
> > It's true LLVM has problems with f80 lowering without x87. I commented it
> > in D112143 and hope D100091 will fix them. We don't need to bother to
> > change it again in future.
>
> Right, but can LLVM lower any x87 80-bit fp operations other than return
> values?
> If it cannot, then I think a source level diagnostic is a good thing to have.
> Otherwise the only handling we have is the codegen crash with "x87 register
> return with x87 disabled" and no source-level context.
No. I checked we are able to lower with changes like below. But it requires
enabling all operations with full test. So emitting diagnostic seems good for
now.
```
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -729,6 +729,8 @@ X86TargetLowering::X86TargetLowering(const X86TargetMachine
&TM,
// FIXME: When the target is 64-bit, STRICT_FP_ROUND will be overwritten
// as Custom.
setOperationAction(ISD::STRICT_FP_ROUND, MVT::f80, Legal);
+ } else {
+ setOperationAction(ISD::FADD, MVT::f80, LibCall);
}
// f128 uses xmm registers, but most operations require libcalls.
--- a/llvm/lib/Target/X86/X86InstrFormats.td
+++ b/llvm/lib/Target/X86/X86InstrFormats.td
@@ -472,6 +472,7 @@ class Ii32PCRel<bits<8> o, Format f, dag outs, dag ins,
string asm,
class FPI<bits<8> o, Format F, dag outs, dag ins, string asm>
: I<o, F, outs, ins, asm, []> {
let Defs = [FPSW];
+ let Predicates = [HasX87];
}
// FpI_ - Floating Point Pseudo Instruction template. Not Predicated.
@@ -479,6 +480,7 @@ class FpI_<dag outs, dag ins, FPFormat fp, list<dag>
pattern>
: PseudoI<outs, ins, pattern> {
let FPForm = fp;
let Defs = [FPSW];
+ let Predicates = [HasX87];
}
// Templates for instructions that use a 16- or 32-bit segmented address as
--- a/llvm/lib/Target/X86/X86InstrInfo.td
+++ b/llvm/lib/Target/X86/X86InstrInfo.td
@@ -929,6 +929,7 @@ def HasAES : Predicate<"Subtarget->hasAES()">;
def HasVAES : Predicate<"Subtarget->hasVAES()">;
def NoVLX_Or_NoVAES : Predicate<"!Subtarget->hasVLX() ||
!Subtarget->hasVAES()">;
def HasFXSR : Predicate<"Subtarget->hasFXSR()">;
+def HasX87 : Predicate<"Subtarget->hasX87()">;
def HasXSAVE : Predicate<"Subtarget->hasXSAVE()">;
def HasXSAVEOPT : Predicate<"Subtarget->hasXSAVEOPT()">;
def HasXSAVEC : Predicate<"Subtarget->hasXSAVEC()">;
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114162/new/
https://reviews.llvm.org/D114162
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
