rogfer01 added a comment.
Overall LGTM. Thanks @khchen!
================
Comment at: clang/include/clang/Basic/riscv_vector.td:175
+ // builtin to C/C++. It is parameter of the unmasked version without VL
+ // operand.
+ list<int> PermuteOperands = [];
----------------
Not sure if we want to clarify that when this list is empty, the permutation is
assumed to be equivalent to `[0, 1, 2, 3, ...]`.
================
Comment at: clang/include/clang/Basic/riscv_vector.td:224
+ IntrinsicTypes = {ResultType, Ops[1]->getType()};
+ Ops[0] = Builder.CreateBitCast(Ops[0],
+ llvm::PointerType::getUnqual(ResultType)); }],
----------------
I think you may want to indent this, and then lines 228 and 248 like you
indented line 252.
================
Comment at: clang/utils/TableGen/RISCVVEmitter.cpp:689
+ skew = 1;
+ for (unsigned i = 0; i < PermuteOperands.size(); ++i) {
+ if (i != PermuteOperands[i])
----------------
These are only suggestions of sanity checks we could do here:
- I understand `PermuteOperand.size()` should be `<=` than `CTypeOrder.size()`.
- Also `PermuteOperands[i] + skew` should be `<` than `CTypeOrder.size()`.
right?
- We could check the result is indeed a permutation (e.g. sorting a copy of
`CTypeOrder` is equivalent to the iota above). This one might be expensive
although the sequences are short, not sure.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98388/new/
https://reviews.llvm.org/D98388
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
