[PATCH] D65118: Analysis: Don't look through aliases when simplifying GEPs.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a comment.

In D65118#1597155 <https://reviews.llvm.org/D65118#1597155>, @jdoerfert wrote:

> In D65118#1596838 <https://reviews.llvm.org/D65118#1596838>, @pcc wrote:
>
> > - The only case in which we can soundly replace an alias with the aliasee is when we know that both have the same value.
>
>
> That is why I thought the "FollowAliases" versions could still look through aliases if they have an "exact definition".
>
> > - However, there's still a benefit in looking past aliases, but only in order to look at the *contents* (not the address) of the global.
> > - For example, the following is valid:
> > 
> >   @a = private constant i8* @x
> >   @b = linkonce_odr alias @a
> >   
> >   define void @foo() {
> >     %x = load i8*, i8** @b
> >     ret i8* %x ; may be replaced with ret i8* @x
> >   }
>
> Mh, this is not what I expected to be valid. To me, this looks like the classical "non-exact-definition" problem that breaks inter-procedural reasoning (@sanjoy, @reames).
>  What happens if I modify the example a bit:
>
> >   @a = private i8 @x
> >   @b = linkonce_odr alias @a
> >   
> >   define i1 @always_true() {
> >     store i8 0, i8* @a
> >     %x = load i8, i8* @b
> >     ... ; @b and @a are not modified here but other globals might be
> >     %y = load i8, i8* @b
> >     ... ; @b and @a are not modified here but other globals might be
> >     %cmp = icmp eq %x, %y
> >     ret i1 %cmp
> >   }
>
> I would assume `always_true` to return "always" `true`, no matter the value of `@b`.
>  However, if we look through `@b` once (say for `%x`) and then replace it at link time
>  with a different alias, couldn't `%y` result in a different value?


Right, and that's why I agree with you that in this case we can't replace `@b` with `@a`. In your example, you're taking an implicit dependency on the global's address by transforming a load from one global into a load from a potentially different global with different contents. When I say the "contents" of a global, what I really mean is the initializer, and only in the case where the global is constant. This, along with the `linkonce_odr` on the alias, guarantees that all copies of the global will have the same contents, which is what allows us to replace the load with the initializer in my example.

>> [...]
>> 
>> - So I think that the ideal final state is that looking past aliases should be opt-in and only allowed when a pass introduces no dependencies on the address of the aliasee.
> 
> So, should we make "NoFollowAliases" the default and add the option/variants that "FollowAliases" instead?

Yes, I think that should be the goal.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65118