[PATCH] D93617: [LoopVectorize] Take argument's size into consideration when computing leader's demanded bits

[guopeilin via Phabricator via llvm-commits]

guopeilin added a comment.

In D93617#2466879 <https://reviews.llvm.org/D93617#2466879>, @nikic wrote:

> Okay, based on your description this *is* a correctness fix, but it's not applied to the right place. The result of the DemandedBits analysis is correct -- demanded bits of the trunc operand are the same as for the trunc result.
>
> As far as I can tell, the actual bug is in computeMinimumValueSizes(). It "successfully" terminates walks at non-Instructions, but that's not right, as the demanded bits for the non-Instruction can be wider. In your avoid-truncate-shift-operands.ll example, note that in `lshr i32 %f, 18`, `%f` has demanded bits outside the low 16 bits, but this fact is lost, because `%f` is an `Argument`. I imagine the same thing happens with constants as well. The fact that `Argument`s are incorrectly truncated seems to be the commonality between all your tests.
>
> As such, I believe a fix here needs to be in computeMinimumValueSizes() -- though possibly DemandedBits also needs to be extended to provide additional information.
>
> A possible way to go about this is to extended the existing DemandedBits code for DeadUses, which currently stores whether certain Uses are completely dead. This could be changed to actually store the demanded bits per-use, not just per-instruction. This would allow you to inspect the demanded bits of arguments and constants as used in a specific instruction. We would have to evaluate the additional cost this would have though.

A new patch updated that takes care of the cases that `Argument incorrectly truncated` when computeMinimumValueSizes. Could you please review this patch.


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

https://reviews.llvm.org/D93617