[PATCH] D23214: [MDA] Treat invariant.start as non-dependence

[Daniel Berlin via llvm-commits]

On Mon, Aug 8, 2016 at 1:51 PM, Sanjoy Das <sanjoy at playingwithpointers.com>
wrote:

> Hi Daniel,
>
> Daniel Berlin wrote:
> > Well, no.
> > If invariant end was readonly, it would value forward past invariant
> > ends, and extend the range in the case it was like this:
> >
> > %a =invariant.start
> > ...
> > invariant.end(%a)
> >
> > <non-memory affecting instructions>
> >
> > %b=invariant.start
> > ...
> > invariant.end(%b)
> >
> > As long as there are no memory writing instructions in the middle,
> > things will believe the two invariant starts produce the same value, and
> > forward it :)
>
> I have to think about it a bit more before I can commit to this, but
> at least intuitively the above seems fine -- if there are no memory
> clobbering instructions between the two ranges then they should be
> merged.



FWIW: I've pretty much not thought about it at all.
The main use case, as far as i remember, was to support specifying
invariantness that otherwise could not be inferred through analysis.
I believe the only use case in my part of the world (C++) is const objects
of various sorts, or write-once objects.

Neither would be broken by this kind of merging.

If we do think the above kind of merging is okay, we should mark
invariant.end readonly as well and teach a small set of things not to hoist
them/eat them, so that we gain the benefits of them not blocking
optimization either (for example, any readnone/readonly function that uses
invariants is still going to be marked readwrite due to the invariant.end)



>   This can even be a valuable optimization in cases where the
> "<non-memory affecting instruction>" bits were hidden behind a call
> that was later inlined.
>
> The way I see it is that invariant_start and invariant_end denote the
> boundaries of a region that can be assumed to not have stores to a
> specific location[1].  To preserve this property we only need to avoid
> stores floating into the region, and for that readonly semantics is
> sufficient for non-atomic accesses (and potentially some kinds of
> atomic accesses too).  Readonly semantics may even be stronger than we
> need, since I think we can allow stores floating *out* of the region.
>

Yes, that sounds right.

>
> I'm not yet fully confident that the above semantics will hold under
> pressure, since it fails the test of "can I write an implementation of
> invariant_start and invariant_end that will give me the guarantees I
> need?".  The best I can do with invariant_start and invariant_end as
> specified is:
>
>   void* invariant_start(iN* ptr) {
>     cell = malloc(sizeof(iN));
>     *(iN*)cell = ptr;
>     return cell;
>   }
>
>   void invariant_end(void* cell, iN* ptr) {
>     assert(*(iN*)cell == *ptr);
>   }
>
>
> which still allows the transform:
>
> x = invariant_start(ptr)
> int val = *ptr;
> print(val);
> invariant_end(x, ptr)
>
> ==>
>
> x = invariant_start(ptr)
> int val_backup = *ptr;
> *ptr = 400;
> *ptr = val_backup;
> int val = *ptr;
> print(val);
> invariant_end(x, ptr)
>
>
> That's not a clear problem (after all, intrinsics are allowed to have
> special semantics that may not be "implementable" in IR), but it is a
> "slightly red" flag.
>


yeah. I guess it would be interesting to know what real world uses for this
currently exist.


>
> [1]: An alternate framing would be: no stores to that specific
>   location change the contained value.
>
> -- Sanjoy
>
>
> >
> > GVN is not quite smart enough yet, but it's about to be.
> >
> > GCC even has testcases that we both eliminate and PRE pure/const
> > (readonly/readnone) calls.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160808/25e325cf/attachment.html>