pcwang-thead added a comment.

In D126461#3566626 <https://reviews.llvm.org/D126461#3566626>, @khchen wrote:

> IMO, if I'm an user, I would not expected intrinsic function will generate 
> the condition code to impact the performance, maybe we need to raise a issue 
> in rvv-intrinsic-doc.
> maybe another way is adding a note in intrinsic document to address that the 
> vl could not be a null pointer.

I understand your concern about impact on performance and I added a test to 
show that the comparison and branch instruction can be optimized if destination 
is known to be not null (which is common in most scenarios using vleff).
For example:

  size_t new_vl;
  v=vleff(..., &new_vl, vl);
  //usage of new_vl.

When doing `instsimplify`, LLVM knows `new_vl` won't be null. Comparison is 
evaluated to be true and branch instruction will be changed to unconditional 
branch (which will be simplified by `simplifycfg`).
So I think there is no impact on performance for most scenarios. For some cases 
that we don't know whether destination is null or not (like using destination 
passed by argument in `vleff.c`, but I think it is not normal usage and IPO may 
help), it is a potential risk that we may end in program crashing if we don't 
do null pointer checking.

> How about the segment load? Does it make sense to add null pointer checking 
> for all argument v0~vN?

That's a good point and I have thought it before. Here is my point but we can 
have more discussion.
I think we don't need null pointer checking for non-tuple segment loads. One of 
the reason is that we have said (but not documented?) that pointers can't be 
null in https://github.com/riscv-non-isa/rvv-intrinsic-doc/issues/95. Besides, 
I think we should treat the output of segment loads as whole structure though 
we have non-tuple intrinsics for now. The reason why we add null pointer 
checking for vleff is because the outputs of vleff are of two different types 
and we just want to ignore the output new vl.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126461

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to