[PATCH] [ARM] Add knowledge of FPU subtarget features to TargetParser

[Renato Golin]

In http://reviews.llvm.org/D10237#183533, @john.brawn wrote:

> That would be a bit of an odd place to do it, as FPUs and triples only indirectly have anything to do with each other - a triple may indirectly cause a default FPU due to the default CPU, but there's no link in the FPU->Triple direction.


That's a good point.

> It's not clear to my why TargetParser isn't the right place for this. You say "The overall design of this class must be as simple as possible", but I don't see why that is. My impression of TargetParser is that it handles target-specific names in a way that means the thing using it doesn't need target-specific information, e.g. you can go from architecture name to default CPU name without needing to know what those names mean. Going from FPU name to a list of subtarget feature flags doesn't seem that different to me.


Now I'm less convinced that this doesn't belong there, but I really dislike the vector solution.

Clang uses std::vector and std::string too liberally, and I don't think that's good enough for LLVM base libraries. The argument that current cpp files already include <vector> is only valid until we start including TargetParser.h in other cpp files that don't. Standard library headers are very nasty indeed.

However, as you said, any other place will be worse than here... except the future TargetTuple class that doesn't exist yet. The new Tuple is like a Triple on steroids, that knows everything about the target, not just CPU and ARCH, but FPU and extensions, OS and vendor with the necessary and unambiguous context.

For now, I guess we can just leave it here and add a big FIXME on "include <vector>", to remove it as soon as features move to Tuple, and the method itself to move this to Tuple once that's created.

Adding the FIXMEs, removing the spurious brackets and moving the enums to the headers, LGTM. Thanks!

cheers,
--renato


REPOSITORY
  rL LLVM

================
Comment at: lib/Target/ARM/AsmParser/ARMAsmParser.cpp:9199
@@ -9261,1 +9198,3 @@
+    STI.ApplyFeatureFlag(Feature);
   }
+  setAvailableFeatures(ComputeAvailableFeatures(STI.getFeatureBits()));
----------------
no need for brackets any more here either.

http://reviews.llvm.org/D10237

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