[PATCH] D141479: [RISCV] Generate march string from target features

[Francesco Petrogalli via Phabricator via llvm-commits]

fpetrogalli accepted this revision.
fpetrogalli added a comment.
This revision is now accepted and ready to land.

LGTM, with some minor observations:

1. You say `The disadvantage is that the generated march string is too tedious.`. That does seems to imply that autogenerating the string is not a good idea? I'd remove it from the commit message.

2. I am still thinking you should reference the bits of the RISCV specs that describe how to create MArch out of the features.

Thank you!

Francesco



================
Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:39
+    if (llvm::RISCVISAInfo::isSupportedExtensionFeature(FeatureName))
+      FeatureVector.push_back((Twine("+") + FeatureName).str());
+  }
----------------
Nit - maybe if you set `std::vector<StringRef> FeatureVector;` you can avoid the `.str()` in here?


================
Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:42
+
+  llvm::Expected<std::unique_ptr<RISCVISAInfo>> ISAInfo =
+      llvm::RISCVISAInfo::parseFeatures(XLen, FeatureVector);
----------------
Nit: maybe alias the type in the body of the function?

```
using ISAInfoTy = llvm::Expected<std::unique_ptr<RISCVISAInfo>>;
ISAInfoTy ISAInfo = ...
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141479