[PATCH] D126461: [RISCV] Extract and store new vl of vleff/vlsegff iff destination isn't null

Wang Pengcheng via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 20 23:58:43 PDT 2022


pcwang-thead added a comment.

In D126461#3597862 <https://reviews.llvm.org/D126461#3597862>, @craig.topper wrote:

> In D126461#3596761 <https://reviews.llvm.org/D126461#3596761>, @reames wrote:
>
>> Despite the comments above, the purpose of this patch remains unclear.
>>
>> Per the draft spec, the relevant wording is:
>> "These instructions execute as a regular load except that they will only take a trap caused by a synchronous exception
>> on element 0. If element 0 raises an exception, vl is not modied, and the trap is taken. If an element > 0 raises an
>> exception, the corresponding trap is not taken, and the vector length vl is reduced to the index of the element that would
>> have raised an exception."
>>
>> Working through the scenario in this patch with the destination being null, the expected result is for a trap to be generated (provided null is unmapped of course), and VL not to be modified.  In order for this change to make any difference in runtime behavior, the value passed must be null (or otherwise guaranteed to fault).  It seems very odd to me that we are modifying code which only runs after an instruction which is guaranteed to fault.  Is there an assumed runtime here which is e.g. restarting execution?
>
> `dst` in the patch description is not the pointer being loaded, it's the pointer of where to store the new_vl. That is only thing being checked for null in this patch.

Thanks for your kind explanation! @craig.topper
I did this change just because previous GCC implementation have done null pointer checking. Some of our codebases crashed when we switched to LLVM because new_vl is set to NULL to ignore output new vl. : ) @reames


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126461



More information about the cfe-commits mailing list