craig.topper added inline comments.
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:714
void RISCVISAInfo::updateImplication() {
+ const StringMap<std::vector<StringRef>> Implications = {
+ {"v", {"zvlsseg", "zvl128b"}},
----------------
I think I'd like to see this as a static data structure rather than building a
StringMap on the fly.
Maybe like
```
static const char *zvl64bimplied[] = { "zvl32b" };
static const char *zvl128bimplied[] = { "zvl64b" };
...
struct ImpliedEntry = {
StringLiteral Name;
ArrayRef<const char*> ImpliedExtensions;
};
static constexpr ImpliedEntry ImpliedTable[] = {
{ "zvl64b", zvl64bimplied },
{ "zvl128b", zvl128implied },
...
};
```
You can then use std::lower_bound to search the ImpliedTable to find the
correct row of ImpliedTable. I haven't tested this. Maybe I'll put up a patch
on the existing V implications as a proof of concept.
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:782
+ if (IsZvlExt) {
+ ExtName.consume_back("b");
+ unsigned ZvlLen;
----------------
I think we should check the return value from consume_back and getAsInteger to
make sure we really parsed what we think we parsed. That will prevent surprises
if a new extension comes along that also starts with "zvl"
================
Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.cpp:136
assert((RVVVectorBitsMin == 0 ||
- (RVVVectorBitsMin >= 128 && RVVVectorBitsMax <= 65536 &&
+ (RVVVectorBitsMin >= 64 && RVVVectorBitsMax <= 65536 &&
isPowerOf2_32(RVVVectorBitsMin))) &&
----------------
This needs the same FIXME as above.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108694/new/
https://reviews.llvm.org/D108694
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
