This will enable the IAS to reject floating point instructions if soft-float is enabled.
Details
Diff Detail
Event Timeline
This, in general, looks good to me. You will probably have an issue with LegalizeDAG.cpp where it uses the value of UseSoftFloat from TargetOptions to determine whether/how to legalize something, but it's being actively worked on.
-eric
I've got a few comments but nothing major. LGTM with those.
lib/Target/Mips/AsmParser/MipsAsmParser.cpp | ||
---|---|---|
90 | I don't think soft-float is an arch related feature. Does '.set mips32' and similar reset it? | |
lib/Target/Mips/Mips32r6InstrInfo.td | ||
655 | We should extend PredicateControl with a SoftFloatPredicates member and add adjectives (HARDFLOAT/SOFTFLOAT?) to set that. | |
lib/Target/Mips/MipsInstrFPU.td | ||
120 | This isn't related to your patch so no change required in this patch: NotFP64bit and IsFP64bit belong in PredicateControl.FGRPredicates, not in PredicateControl.AdditionalPredicates. The FGR_64 and FGR_32 adjectives can be used to do this. There seems to be a few instances of this kind of thing left. | |
408–409 | Nit: Why break the line? | |
438 |
They require hardfloat. |
I've got a few comments but nothing major. LGTM with those.
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:90
@@ -90,1 +89,3 @@
+ Mips::FeatureFP64Bit | Mips::FeatureGP64Bit |
Mips::FeatureNaN2008 |
+ Mips::FeatureSoftFloat;
I don't think soft-float is an arch related feature. Does '.set mips32'
and similar reset it?
In general the .set options are supposed to be as independent as possible
there is unfortunate historic entanglement in binutils which means that
we have to allow .set mips<blah> to automatically set gp and fp defaults.
However, with fp=xx I believe the behaviour is that .set mips<blah>
does not force a specific fp setting when the current value is xx. The
other .set options only update one feature even if that produces a
slightly unusual overall configuration.
Thanks,
Matthew
Removed redundant comment and newline.
Removed soft-float from AllArchRelatedMask.
Extended PredicateControl with a HardFloatPredicate, which is set by using HARDFLOAT.
Daniel, do you mind taking a look at that last change ?
I'm not sure if it fully addresses your concerns.
So this behaviour changes between fp=xx and other fp values? We'll need to look into that but I think it's something for a follow-on patch.
I spotted four def's that seem to have lost IsNotSoftFloat but otherwise it LGTM.
The only unresolved bit I can see is that we need to switch AdditionalPredicates<[NotFP64bit]> for FGR_32 and AdditionalPredicates<[IsFP64bit]> for FGR_64 but that's a change for another patch.
lib/Target/Mips/MipsInstrFPU.td | ||
---|---|---|
570–590 | We seem to have lost IsNotSoftFloat on these four def's. I don't see it in the base class. |
In general the .set options are supposed to be as independent as
possible there is unfortunate historic entanglement in binutils which
means that we have to allow .set mips<blah> to automatically set gpand fp defaults.
However, with fp=xx I believe the behaviour is that .set mips<blah>
does not force a specific fp setting when the current value is xx. The
other .set options only update one feature even if that produces a
slightly unusual overall configuration.Thanks,
MatthewSo this behaviour changes between fp=xx and other fp values? We'll need
to look into that but I think it's something for a follow-on patch.
Yes, that's correct.
Matthew
I don't think soft-float is an arch related feature. Does '.set mips32' and similar reset it?