craig.topper added inline comments.
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:70
+ // passing to the BUILTIN() macro in Builtins.def.
+ std::string builtin_str() const { return BuiltinStr; }
+
----------------
Return a const std::string & or a StringRef.
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:85
+ bool isScalar() const {
+ return (Vscale.hasValue() && Vscale.getValue() == 0);
+ }
----------------
Drop parentheses
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:154
+ // Return the architecture preprocessor definitions.
+ static SmallVector<std::string> getExtStrings(uint8_t Extensions);
+
----------------
Does this need to be in Intrinsic? Can it just be in the emitter class?
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:984
+ StringRef MangledSuffix = R->getValueAsString("MangledSuffix");
+ std::string Prototypes = R->getValueAsString("Prototype").data();
+ StringRef TypeRange = R->getValueAsString("TypeRange");
----------------
Use str() not data().
But I'm not sure why it can't just a be StringRef?
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:997
+
+ // Parse prototype and create a list of primitve type with transformers
+ // (operand) in ProtoSeq. ProtoSeq[0] is output operand.
----------------
primitive*
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1070
+ // search first
+ if (LegalTypes.count(Idx)) {
+ return Optional<RVVTypePtr>(LegalTypes[Idx]);
----------------
Use LegalTypes.find() so you don't have to two look ups.
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1076
+ // compute type and record the result
+ auto T = std::make_shared<RVVType>(BT, LMUL, Proto);
+ if (T->isValid()) {
----------------
Does this need to be shared_ptr? Can we just arrange for LegalTypes to own the
types and give every one else a pointer? LegalTypes would just need to outlive
the references to the types.
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1102
+ if (ExtStrings.size()) {
+ std::string ArchMacro = std::accumulate(
+ ExtStrings.begin() + 1, ExtStrings.end(), "(" + ExtStrings[0] + ")",
----------------
Can we just stream this out to OS instead of accumulating a string before
streaming?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95016/new/
https://reviews.llvm.org/D95016
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
