Expose the list of available features in MCSubtargetInfo

[johanengelen]

Summary

I have not found a way to obtain a list of features available for the target machine. MCSubtargetInfo provides the getFeatureBits() method, but I have not found a way to translate the FeatureBitset to feature names (e.g. "sse2", "avx") or their description. MCSubtargetInfo does contain a reference to that information but does not expose that to the user.
This patch exposes the private ProcFeatures list to user code, similar to exposing the FeatureBits.

I intend to use this in our front-end (LDC) to check whether a feature is enabled or not:

auto *mcinfo = gTargetMachine->getMCSubtargetInfo();
auto featbits = mcinfo->getFeatureBits();
auto featTable = mcinfo->getProcFeatures();
for (auto &feat : featTable) {
  if ((featbits & feat.Value) == feat.Value)
    //check if feat.Key is equal to the feature we seek
}

Thanks for reviewing.

[johanengelen]

include/llvm/MC/MCSubtargetInfo.h

85

Should the return type be const?

[johanengelen]

A friendly ping.

[mkuper]

Sorry for the delay,
I don't think I'm the right person to review this, I'm not really an expert on MC, the reason my name is all over MCSTI it is because I had to change the underlying data structure for the bits.

In any case, my 2 cents is that this doesn't seem right. I don't think we want to expose SubtargetFeatureKV externally. Perhaps it would be better to be able to query MCSubtargetInfo for a feature directly - similarly to the way we do ToggleFeature today.

A separate issue is that I don't know what the policy is about adding interfaces to MC that aren't used by anything in-tree. That is, I'm not even sure MCSubtargetInfo is intended to interact with FEs. This is a question for grosbach.

[johanengelen]

I don't know how I missed "ToggleFeature", thanks very much for pointing me in that direction!
With the current interface, I think I can already do what I want:

auto *mcinfo = gTargetMachine->getMCSubtargetInfo();
auto SavedFeatbits = mcinfo->getFeatureBits();
auto NewFeatbits = mcinfo->ApplyFeatureFlag("feature to query");
bool FeatureFound = (SavedFeatbits == NewFeatbits);
mcinfo->setFeatureBits(SavedFeatbits);

Do you think it still makes sense to add a bool MCSubtargetInfo::QueryFeatureFlag(StringRef FS) ?

[johanengelen]

(yes I think it does make sense, because the necessary const_cast is very ugly)

[echristo]

I don't get it, what are you trying to do again? Can you show an example of the use you're looking for?

[johanengelen]

I want to know whether a certain feature (say, "avx") is enabled for the current target.

This works for us though:

bool traitsTargetHasFeature(Dstring feature) {
   auto feat = llvm::StringRef(feature.ptr, feature.len);
 
   // Copy MCSubtargetInfo so we can modify it.
   llvm::MCSubtargetInfo mcinfo = *gTargetMachine->getMCSubtargetInfo();
   auto savedFeatbits = mcinfo.getFeatureBits();
 
   // Nothing will change if the feature string is not recognized or if the
   // feature is disabled.
   {
     auto newFeatbits = mcinfo.ApplyFeatureFlag(("-" + feat).str());
     if (savedFeatbits == newFeatbits) {
       return false;
     }
     mcinfo.setFeatureBits(savedFeatbits);
   }
   {
     // Now that unrecognized feature strings are excluded,
     // nothing will change iff the feature and its implied features are enabled.
     auto newFeatbits = mcinfo.ApplyFeatureFlag(("+" + feat).str());
     return savedFeatbits == newFeatbits;
   }
 }

include/llvm/MC/MCSubtargetInfo.h

public:
   }
 
   /// setFeatureBits - Set the feature bits.
   ///
   void setFeatureBits(const FeatureBitset &FeatureBits_) {
     FeatureBits = FeatureBits_;
   }
 
+  /// getProcFeatures - Return the subtarget feature list.
+  ///
+  /// Use this list in conjunction with getFeatureBits() to get which features
+  /// are enabled for the current CPU.
+  ArrayRef<SubtargetFeatureKV> getProcFeatures() const {
+    return ProcFeatures;
+  }
+
 protected:
   /// Initialize the scheduling model and feature bits.
   ///
   /// FIXME: Find a way to stick this in the constructor, since it should only
   /// be called during initialization.
   void InitMCProcessorInfo(StringRef CPU, StringRef FS);
 
 public: