[PATCH] D93006: [RISCV] Initial support for RVV intrinsic

[PeiHsiangHung via Phabricator via llvm-commits]

NickHung added inline comments.


================
Comment at: llvm/test/CodeGen/RISCV/rvv/intrinsic-load-add-store-32.ll:10
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    vsetvli a3, zero, e32,m1,tu,mu
+; CHECK-NEXT:    vle32.v v25, (a1)
----------------
craig.topper wrote:
> NickHung wrote:
> > craig.topper wrote:
> > > This is setting the VL to VLmax which isn't what the spec wants. It should get the value from the previous vsetvl intrinsic or maybe the previous intrinsic that had a vl argument.
> > > 
> > > Our internal implementation has been implementing the intrinsics without vl by inserting a readvl intrinsic and a call to the intrisics that take vl. But we've been finding issues with this. The readvl is acting as an optimization barrier. It also doesn't have any ordering in IR with respect to intrinsics that have a vl argument unless we mark all intrinsics has having side effects.
> > > 
> > > What are your thoughts on this?
> > In our internal implementation, intrinsic without vl should insert "setvli zero, zero, e32,m1,tu,mu" to kepp the current VL. 
> > I didn't touch any code in D89449 in order to respect the authors.
> > Please check out load-add-store-32.ll,  it has the same vsetvl which changes the current vl.
> > 
> > Our thought is to follow the RVV intrinsic documentation 
> >  https://github.com/riscv/rvv-intrinsic-doc. So, we provide all kinds of intrinsics with and without vl. 
> > 
> > Thanks for sharing your ideas with us.
> > 
> load-add-store-32.ll is intentionally using vlmax. I believe it was intended to model a scalable vector style similar to ARM SVE where there is no VL and instead a whole register of a size only known at runtime is used.
Thanks for the explanation. Your thought is correct.
As I mentioned in D89449, the initial infrastructure should be shared by RVV intrinsic and IR nodes. 

So, we know it should do a little change to support both RVV intrinsic and IR nodes for distinct programming scenarios.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93006/new/

https://reviews.llvm.org/D93006