[PATCH] D93617: [DemandedBits] Add a whitelist when computing demanded bits of Trunc Instruction

guopeilin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 23 04:21:07 PST 2020


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.

Thanks  for reviewing. 
And I still get confused in some places. One is that the DemandedBits.cpp seems compute the demandedbits per-instruction, that is it `"successfully" terminates walks at non-Instructions`, too.
I guess the way computing demanded-bits per-instruction is not compatible with the way `truncateToMinimalBitwidths()` truncating the operands for every instruction in `MinBWs`. 
So if we still want keep truncateToMinimalBitwidths(), then we may need modify function computeMinimumValueSizes() to take Arguments into consideration, Or if we still want to compute demanded bits in per-instruction way, then is it a possible solution that we be more conservative in function truncateToMinimalBitwidths(), that is we add some patterns, if match then avoid truncating?


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

https://reviews.llvm.org/D93617



More information about the llvm-commits mailing list