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

[Daniel Berlin via llvm-commits]

On Fri, Aug 5, 2016 at 12:40 PM, Sanjoy Das <sanjoy at playingwithpointers.com>
wrote:

> Hi Daniel,
>
> Daniel Berlin wrote:
> > I really want to say yes, but ....
> >
> > 1. Your argument is identical for the other intrisics right above it.
> > None of them touch memory in practice.
> > 2. MDA is deliberately returning it as a Dep so that things know where
> > the invariants start and end (again, same with lifetime markers).
> > I realize this is not ideal (and in fact, sucks), but things depend on
> it.
> >
> > 3. Without this, you will enable hoisting that is not allowed, if things
> > use MDA to perform hoisting.
> >
> > You cannot move the invariant start and end calls, and MDA saying they
> > are read-only will enable them to be moved.
>
> Are you saying that they'll get speculated out of control flow?


Yes, i'm saying the  mechanism currently used to keep them from moving is
to say they read/write memory.

That
> does not seem safe -- how does LLVM know that they won't segfault?
>


I'm not sure i understand.
These are invariant start and end markers, they generate no real code.
If you move them, the problem is that the definition of invariant.start
says:
"This intrinsic indicates that until an llvm.invariant.end that uses the
return value, the referenced memory location is constant and unchanging.
"

So if you hoist/sink start or end , you change the bounds of where the
memory is unchanging.

You could fix this, of course, by having the invariant.start and
invariant.end calls return pointers, and all things that are invariant use
those pointers (or pointers derived from those pointers).  IE express the
dependence in SSA directly.

Then you can move the start/end calls wherever they are valid (IE wherever
they are dominated by their operands) and it will not change the semantic
effect.

But that's not the current design.




> > We last tried this by saying they did not touch memory (which is
> > correct), and peter had to revert the set of changes that gave them "The
> > correct answer" because it broke on real code.
>
> Do you happen to remember any relevant revision numbers / zilla bugs
> for this?
>
> The revert was: "[llvm] r270823 - MemorySSA: Revert r269678 and r268068;
replace with special casing in MemorySSA.
"

This is when we tried this with assume
Look for emails from pcc around the time you see "[PATCH] D20653: LICM: Do
not sink or hoist assume intrinsic calls."

This was a set of followups from peter to try to starting fixing places
that tried to hoist them anyway, and it turns out ... it's not a pretty
path.
;)


> -- Sanjoy
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160805/c4016ee6/attachment.html>