[PATCH] D130895: [RISCV] Make VL choosing for a splat-like VMV based on its users

[Anton Sidorenko via Phabricator via llvm-commits]

asi-sc added a comment.

In D130895#3922106 <https://reviews.llvm.org/D130895#3922106>, @reames wrote:

> I spent some time this morning creating a minimal version of this patch.  This change is still too complicated, and I'm unconvinced of it's correctness.  Please see https://reviews.llvm.org/D137856.
>
> While doing this, I realize this change is definitely incorrect.  Consider the following test case:
> setvli x1, 5, <fixed lmul, flags>
> vmv.v.i v8, -1
> setvli x1, 1, <fixed lmul, flags>
> vle8 v8, (a5)  // memory contains an -1 vector
> setvli x1, 5, <fixed lmul, flags>
> vmseq v9, v8, -1
>
> In this case, vle8 stands in for any instruction with a merge operand.   The fact it's a load is not meant to be important.
>
> The required semantics of this program is to produce a mask with five ones - regardless of the TA/TU status of the vle8.  TA allows either the original value or -1, and in this case, both choices result in the same result.
>
> However, this patch would produce the following:
> setvli x1, 1, <fixed lmul, flags>
> vmv.v.i v8, -1
> setvli x1, 1, <fixed lmul, flags>
> vle8 v8, (a5)  // memory contains an -1 vector
> setvli x1, 5, <fixed lmul, flags>
> vmseq v9, v8, -1
>
> In this case, lanes 1 through 4 (inclusive) are undefined, and may result in a mask without those lanes set.  As such, the transform is unsound as currently implemented.

@reames , thanks for reviewing it so thoroughly! However, the test case you provided doesn't convince me in the incorrectness of this change. 
(1) If the instruction with a merge operand (`vle`) has TU policy, we won't shrink VL and the code remains untouched. 
(2) The instruction has TA policy. In this case, I don't think we can rely on the values written to the tail. According to the spec:

> The value of all 1s instead of all 0s was chosen for the overwrite value to discourage software developers from depending on the value written.

For me it means "you must never rely on the concrete value written to the tail if TA policy is used". Am I wrong? Do we really expect to see some concrete values in the tail of TA instruction when doing compiler transformations? If so, we should just reject shrinking VL for any merge operands.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130895