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

[Piyou Chen via Phabricator via cfe-commits]

BeMg 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:
> 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());
> ```
Thank you for further explanation. I think getTargetFeatureFromOneExt can do this job.

```
std::string TargetFeature = llvm::RISCVISAInfo::getTargetFeatureFromOneExt(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