[PATCH] D151730: [RISCV] Support target attribute for function

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.


================
Comment at: clang/lib/Basic/Targets/RISCV.cpp:385
+          StringRef ExtName = Ext.substr(1);
+          if (llvm::RISCVISAInfo::isSupportedExtensionWithVersion(ExtName) ||
+              llvm::RISCVISAInfo::isSupportedExtension(ExtName))
----------------
craig.topper wrote:
> I wonder if we could encapsulate this `if` and the 3 calls into RISCVISAInfo into a single function in RISCVISAInfo?
> 
> Basically we want to know if an extension that may or may not have a version, is a valid extension and if so what is the target feature name for it. We could have one function that does all that and returns the target feature name or an empty string.
I think I explained my idea poorly. I was wondering about having a function that allowed us to write

```
std::string TargetFeature = llvm::RISCVISAInfo::getTargetFeatureForExtension(ExtName);
if (!TargetFeature.empty())
  Ret.Features.push_back(Ext.front() + TargetFeature);
else
  Ret.Features.push_back(Ext.str());
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151730/new/

https://reviews.llvm.org/D151730