[PATCH] D145088: [RISCV] Add attribute(riscv_rvv_vector_bits(N)) based on AArch64 arm_sve_vector_bits.

[Craig Topper via Phabricator via cfe-commits]

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