[PATCH] D85592: [Attributor] Add flag for undef value to the state of AAPotentialValues

Shinji Okumura via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 24 15:16:07 PDT 2020


okura added a comment.

In D85592#2221775 <https://reviews.llvm.org/D85592#2221775>, @jdoerfert wrote:

> In D85592#2221557 <https://reviews.llvm.org/D85592#2221557>, @fhahn wrote:
>
>> I am not sure if the following scenario is possible, but if it is it might be problematic when undef is unified with multiple values. E.g. we found that we have the following potential values `0, 1, undef`. If we now assume this is `0, 1`, we might simplify `and %x, 3` to `%x`. But that is wrong, if `%x` is `undef`. Note that simplifications that result in a constant should be fine either way. As I said, not sure if the potential values could be used like that (possibly in the future).
>
> It doesn't seem to be a problem now. Two results from the IRC discussion:
>
> 1. We need a test for the above situation, to make sure we don't regress this.
> 2. We should make sure when we merge the possible values with undef properly, e.g., if you intersect two sets of possible values {0,1,undef} and {0, 7} then we should get {0, 7}.

I'm not sure we have to take care of intersection. Currently, there is `PotentialValueState::intersectWith` but is not used anywhere. I am not able to come up with a case we need it, and I think it should be deleted.


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

https://reviews.llvm.org/D85592



More information about the llvm-commits mailing list