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

[Daniel Berlin via llvm-commits]

to be 100% concrete, these are what i think you would have to do to move
forward sanely with MemDep (MemSSA didn't built the API this way, so it
doesn't have these issues), and still provide the existing users what they
are using it for.

1. Either verify that nobody is depending on MDA to give them clobbers for
invariant start/end (maybe they are all dead? Things definitely depend on
it for lifetime.start/lifetime.end clobbers).
Assuming some still exist, provide an API that, given a load, tells where
starts being invariant/ends being invariant.  Change existing users to use
that.

2. (Optional bonus): Do the same for lifetime markers, which are the other
intrinsics here marked the same way.

3. Handle the issues of hoisting/sinking *the intrinsic calls themselves*
somehow, as moving them breaks stuff since you are extending the
invariantness range

4. (Optional bonus) Same for lifetime.start/end

5. (Bonus points) Think about the issue of hoisting/sinking *operations
past them* (IE if you hoist a load above the invariant marker, it's now not
invariant).  Not sure what should happen here, if anything.  But it does
mean whatever attribute they get marked with may not want to block hoisting
past.


On Fri, Aug 5, 2016 at 12:48 PM, Daniel Berlin <dberlin at dberlin.org> wrote:

>
>
> On Fri, Aug 5, 2016 at 12:40 PM, Sanjoy Das <sanjoy at playingwithpointers.co
> m> 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/b227f831/attachment.html>