[PATCH] D64258: [InferFuncAttributes] extend 'dereferenceable' attribute based on loads

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D64258#1572004 <https://reviews.llvm.org/D64258#1572004>, @spatel wrote:

> In D64258#1571981 <https://reviews.llvm.org/D64258#1571981>, @jdoerfert wrote:
>
> > The direction of this makes total sense and we will need it. However this shoulnd't be here (wrt. the file/pass).
> >
> > Assuming we want this right right now, it should life in FunctionAttrs.cpp. Assuming we want to do it "right" it should become part of the Attributor framework.
> >
> > The early prototype of the "deref-or-null" abstract attribute already had this functionality, see https://reviews.llvm.org/D59202#C1381429NL1995, and the test case https://reviews.llvm.org/D59202#change-FJbHx7N4s6ye . For the new Attributor, dereferenceable-or-null has not yet been ported and the transfer of "close by information" is not part of the new model. Both things are going to change soon.
>
>
> Thanks for taking a look! I was just about to add you as a reviewer. I know you are working on a major overhaul of this functionality, but I have not gotten a chance to look at those patches.
>  The reason I did not put this into FunctionAttrs.cpp is because that's currently too late to catch the motivating example from PR21780 using the default opt pass pipeline. Ie, -instcombine runs before -functionattrs and kills the loads before we have a chance to update the arguments. I would like to get this in soon to make the next clang major release, so this seemed like the patch of least resistance. :)
>  If there's a better way, I can certainly try to make it happen.


Thanks for thinking of me ;) And again, I think this is an important change we need!

The Attributor is in tree and, if enabled, it is run very early (as I very very strongly believe it should). I think we can get the Attributor enabled for the next release (maybe with a low iteration count and restrictions on the attributes we derive). Now there are two missing parts to get this functionality into the Attributor in a decent way:

1. A generic way to "look around for existing information" (more on this below).
2. The abstract attribute for dereferenceability(_or_null) that makes use of 1) and potentially performs usual deduction.

Implementing 2) is fairly easy. It should not take long to create the boilerplate if we only want to rely on the deduction through 1). Also, the logic is already in this patch (and the old prototype).
Regarding 1):
I was going to work on this once I found some free cycles but I could do it now if we decide to go this way. The idea is that you specify a program point PP (=instruction) and a callback. The callback is then automatically applied to all instruction which have to be executed when PP is also reached, either before or after. I would like this to be an abstract interface from the get-go but I am also willing to provide the interface and the initial implementation that will at least suffice for this use case. It should then be used from the `AbstractAttribute::initialize` and `AbstractAttribute::updateImpl` method of the abstract attribute for the dereferenceable attribute (and others later as well).

P.S. You should be aware of the change to dereferenceability that is going to happen very soon, see D61652 <https://reviews.llvm.org/D61652> and D63243 <https://reviews.llvm.org/D63243> (I'm still fixing that one).


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

https://reviews.llvm.org/D64258