[PATCH] [mips] Add the SoftFloat MipsSubtarget feature.

Daniel Sanders daniel.sanders at imgtec.com
Wed May 6 09:04:37 PDT 2015


In http://reviews.llvm.org/D9053#158990, @mpf wrote:

> > 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


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.

In http://reviews.llvm.org/D9053#162561, @tomatabacu wrote:

> 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.


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.


================
Comment at: lib/Target/Mips/MipsInstrFPU.td:536-556
@@ -530,23 +535,23 @@
 // allocation.
 class BuildPairF64Base<RegisterOperand RO> :
   PseudoSE<(outs RO:$dst), (ins GPR32Opnd:$lo, GPR32Opnd:$hi),
            [(set RO:$dst, (MipsBuildPairF64 GPR32Opnd:$lo, GPR32Opnd:$hi))]>;
 
 def BuildPairF64 : BuildPairF64Base<AFGR64Opnd>,
                    AdditionalRequires<[NotFP64bit]>;
 def BuildPairF64_64 : BuildPairF64Base<FGR64Opnd>,
                       AdditionalRequires<[IsFP64bit]>;
 
 // This pseudo instr gets expanded into 2 mfc1 instrs after register
 // allocation.
 // if n is 0, lower part of src is extracted.
 // if n is 1, higher part of src is extracted.
 class ExtractElementF64Base<RegisterOperand RO> :
   PseudoSE<(outs GPR32Opnd:$dst), (ins RO:$src, i32imm:$n),
            [(set GPR32Opnd:$dst, (MipsExtractElementF64 RO:$src, imm:$n))]>;
 
 def ExtractElementF64 : ExtractElementF64Base<AFGR64Opnd>,
                         AdditionalRequires<[NotFP64bit]>;
 def ExtractElementF64_64 : ExtractElementF64Base<FGR64Opnd>,
                            AdditionalRequires<[IsFP64bit]>;
 
----------------
We seem to have lost IsNotSoftFloat on these four def's. I don't see it in the base class.

http://reviews.llvm.org/D9053

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list