[PATCH] D19265: [Sparc] Add Soft Float support

[Eric Christopher via llvm-commits]

echristo added inline comments.

================
Comment at: lib/Target/Sparc/SparcSubtarget.cpp:54
@@ -52,3 +53,3 @@
 SparcSubtarget::SparcSubtarget(const Triple &TT, const std::string &CPU,
-                               const std::string &FS, TargetMachine &TM,
+                               const std::string &FS, const TargetMachine &TM,
                                bool is64Bit)
----------------
Can you do this constification in a separate patch? (Feel free to just submit it).

================
Comment at: lib/Target/Sparc/SparcTargetMachine.cpp:87
@@ +86,3 @@
+  // function, so we can enable it as a subtarget feature.                      
+  bool softFloat =                                                              
+      F.hasFnAttribute("use-soft-float") &&                                     
----------------
jacob_hansen wrote:
> jyknight wrote:
> > I wonder what the point of having this redundant way of specifying soft-float via "use-soft-float" is. Clang seems to emit *both* use-soft-float=true and +soft-float. Hopefully this isn't needed these days. @echristo?
> I was wondering as well actually, didn't really find an answer to why this might be, but all targets seem to do this at the moment.
It's redundant as a subtarget feature for soft float wasn't across all targets yet, but if someone is willing to do that cleanup we can get rid of it. (Should do this in a separate patch as I know there are out of tree users etc...)

================
Comment at: lib/Target/Sparc/SparcTargetMachine.h:25
@@ -24,2 +24,3 @@
   std::unique_ptr<TargetLoweringObjectFile> TLOF;
   SparcSubtarget Subtarget;
+  mutable StringMap<std::unique_ptr<SparcSubtarget>> SubtargetMap;
----------------
jacob_hansen wrote:
> jyknight wrote:
> > Pretty sure this ought to be deleted while adding the SubtargetMap.
> I think you are right. Currently I use that object on line 100 of SparcTargetMachine.cpp to determine whether the target is currently 32 or 64 bit (using is64Bit()). Should I just create a new boolean variable here to determine this value instead of using the Subtarget object, or is there a better way?
This should be removed for sure. You should determine 32 or 64-bit ness in the TargetMachine based on what information you have in the TargetMachine configuration rather than the other. It can't change on a subtarget specific basis anyhow.


http://reviews.llvm.org/D19265