craig.topper added inline comments.
================ Comment at: clang/include/clang/AST/ASTContext.h:2262 + /// Return true if the given vector types are lax-compatible RVV vector types, + /// false otherwise. + bool areLaxCompatibleRVVTypes(QualType FirstType, QualType SecondType); ---------------- erichkeane wrote: > craig.topper wrote: > > erichkeane wrote: > > > Same here, what is 'lax compatible' mean here? And RVV? > > Do you have the same comment for the AArch64 equivalent on line 2252? > I do NOW! I mentioned -flax-vector-conversions=. Is that sufficient? ================ Comment at: clang/lib/CodeGen/TargetInfo.cpp:11390 + ResType = llvm::ScalableVectorType::get( + llvm::Type::getIntNTy(getVMContext(), XLen), 64 / XLen); + break; ---------------- erichkeane wrote: > craig.topper wrote: > > jrtc27 wrote: > > > erichkeane wrote: > > > > craig.topper wrote: > > > > > erichkeane wrote: > > > > > > Where is 'XLen' from here? > > > > > It's a member of RISCVABIInfo. It's 64 for riscv64 triple and 32 for > > > > > riscv32 triple. > > > > Well, the name is awful :) I'd probably suggest a re-name and hiding > > > > it behind a function call (since that way it can be done on the triple, > > > > rather than an initialized variable perhaps?), but I'm not really in > > > > charge of this target info. > > > It's not for anyone in the RISC-V space, since it is defined by the > > > architecture and used pervasively (and means the X register LENgth, i.e. > > > how many bits in the x0-x31 GPRs). Using anything else in a RISC-V ABI > > > context would be worse from a RISC-V perspective. In a random LLVM > > > checkout I have I see 1118 instances of `/xlen/i` in > > > llvm/lib/Target/RISCV alone. > > It's the term in the RISC-V spec for the size of our integer registers. > > Anyone working on RISC-V should be familiar with it. > Based on Jessica's post, perhaps it is not an issue. Just was jarring to see > something as impenetrable. I'd perhaps suggest something like `XRegisterLen` > to make it clear what 'X' is, but just a suggestion for the next folks who > are finding their way into contributing patches, despite perhaps not yet > being RISCV experts. I rewrote it using ConvertType and getTypeSize. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145088/new/ https://reviews.llvm.org/D145088 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
