jrtc27 added inline comments.
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:835
+ llvm::RISCVISAInfo::filterISAStrings(Features);
+ std::vector<llvm::Metadata *> Ops;
+ if (Features.empty()) {
----------------
Why is this building a list? Just use a string so it's in the same format as
the function attributes? We already have parsers for that. Yes, it means you do
a small amount of redundant work but having a single format in the IR for
representing the same thing in every place is better than having multiple
different ways of representing the same thing.
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:843
+ }
+ getModule().addModuleFlag(llvm::Module::AppendUnique, "riscv-isa-features",
+ llvm::MDNode::get(Ctx, Ops));
----------------
Just like we call it target-abi not riscv-abi, this should just be called
target-features.
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:132-142
+void RISCVISAInfo::filterISAStrings(std::vector<std::string> &Features) {
+ erase_if(Features, [](std::string &Feature) {
+ StringRef ExtName(Feature);
+ assert(ExtName.size() > 1 && (ExtName[0] == '+' || ExtName[0] == '-'));
+ ExtName = ExtName.drop_front(1); // Drop '+'
+ stripExperimentalPrefix(ExtName);
+ if (filterSupportedExtensionInfosByName(ExtName).empty())
----------------
Why do we need to do any filtering? Just emit the whole thing.
================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp:53-60
+ emitTargetAttributes(ISAInfo);
+}
+void RISCVTargetStreamer::emitTargetAttributes(const RISCVISAInfo &ISAInfo) {
+ if (ISAInfo.hasExtension("e"))
+ emitAttribute(RISCVAttrs::STACK_ALIGN, RISCVAttrs::ALIGN_4);
+ else
----------------
This is unrelated
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D106347/new/
https://reviews.llvm.org/D106347
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
