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

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.
Herald added a subscriber: wangpc.


================
Comment at: clang/lib/Basic/Targets/RISCV.cpp:369
+    Feature = Feature.trim();
+    StringRef Attrstring = Feature.split("=").second.trim();
+
----------------
Attrstring -> AttrString


================
Comment at: clang/lib/Basic/Targets/RISCV.cpp:384
+
+          std::string ExtName = Ext.substr(1).str();
+          if (llvm::RISCVISAInfo::isSupportedExtensionWithVersion(ExtName) ||
----------------
Why std::string, don't all 3 functions its passed to take a StringRef?


================
Comment at: clang/lib/Basic/Targets/RISCV.cpp:391
+          else
+            llvm::errs() << "invalid extension name " << Ext
+                         << ". Skip it.\n";
----------------
We can't write directly to llvm::errs(). It needs to go through clang's Diagnostics code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151730