khchen added inline comments.
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:270
+ IsPointer(false), IsSize_t(false), IsPtrdiff_t(false),
+ ElementBitwidth(~0U), Scale(0) {
+ applyBasicType();
----------------
craig.topper wrote:
> Why is ElementBitwidth default ~0. Wouldn't 0 also be an invalid value?
I followed SVE did, but you are right, 0 is better.
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:714
+ // Emit function arguments
+ if (CTypeOrder.size() > 1) {
+ OS << InputTypes[CTypeOrder[0]]->type_str() + " op0";
----------------
craig.topper wrote:
> Why is this > 1 and not >= 1 or !CTypeOrder.empty()?
Thanks, it's bug when I refactor code from output input vector to input only
vector...
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:757
+ OS << "#error \"Vector intrinsics require the vector extension.\"\n";
+ OS << "#else\n\n";
+
----------------
craig.topper wrote:
> Can we just #endif here instead of the #else? If the error is emitted the
> preprocessor should stop and not process the rest of the file. Then we don't
> need to close it at the bottom of the file.
Good point. Thanks. I should think more when I copied code from SVE.
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:873
+ // Dump switch body when the ir name changes from previous iteration.
+ RVVIntrinsic *PrevDef = Defs.begin()->get();
+ for (auto &Def : Defs) {
----------------
craig.topper wrote:
> Can we remember the PrevIRName StringRef instead?
I don't think so. We need `PrevDef` to emitCodeGenSwitchBody.
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:1027
+ return false;
+ bool NeedOR = false;
+ OS << "#if";
----------------
craig.topper wrote:
> I think you can use ListSeparator for this. It keeps track of the separator
> string and whether the first item has been printed. It's most often used with
> loops, but it should work here. I think there are many examples uses in
> llvm/utils/TableGen
thanks, it's more elegant.
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
