[PATCH] D12310: Introducing llvm.invariant.group.barrier intrinsic & Global Opt handling

[Piotr Padlewski via llvm-commits]

On Tue, Sep 8, 2015 at 11:32 AM, Nick Lewycky <nlewycky at google.com> wrote:

> On 8 September 2015 at 11:28, Piotr Padlewski <prazek at google.com> wrote:
>
>> Prazek marked an inline comment as done.
>>
>> ================
>> Comment at: lib/CodeGen/CodeGenPrepare.cpp:1414-1417
>> @@ -1413,2 +1413,6 @@
>>      }
>> +    case Intrinsic::invariant_group_barrier:
>> +      II->replaceAllUsesWith(II->getArgOperand(0));
>> +      II->eraseFromParent();
>> +      return true;
>>      }
>> ----------------
>> nlewycky wrote:
>> > Do we also discard the !invariant.group metadata on instructions?
>> >
>> > The backend has pointers to the Value*'s, it would be bad if they used
>> the !invariant.group markers but all the invariant.group.barrier calls had
>> been removed.
>> The question is, does it make any difference? What I understand, is that
>> CodeGenPrepare is done just before machine code generation, which means
>> that additional metadata in load/stores doesn't make any difference.
>>
>
> Today it does not. My point is that machine code generation does look at
> Value*. It does things like alias analysis on the IR. Leaving the
> !invariant.group there is a trap for a future developer.
>

Ok, I will get rid of it, but it is not part of this review.
http://reviews.llvm.org/D12026


> ================
>> Comment at: lib/Transforms/IPO/GlobalOpt.cpp:2507-2508
>> @@ -2506,1 +2506,4 @@
>>            continue;
>> +        } else if (II->getIntrinsicID() ==
>> Intrinsic::invariant_group_barrier) {
>> +          setVal(II, getVal(II->getOperand(0)));
>> +          DEBUG(dbgs() << "Passing through invariant.group.barrier
>> intrinsic.\n");
>> ----------------
>> nlewycky wrote:
>> > Why is this safe? We have
>> >   %p2 = call @invariant.group.barrier(%p1)
>> > and we set that %p2 just plain *is* %p1. Then nothing happens until
>> evaluation completes and we commit the changes, which means replacing %p2
>> with %p1. At the time, why won't that operation miscompile?
>> >
>> > I can think of two possible reasons, one is that %p1 may be constrained
>> (like, it may be a ConstantExpr) and the other is that we may know that
>> there are no remaining loads of either %p1 or %p2 with !invariant.group on
>> them.
>> >
>> > In any event, please answer in the form of a comment in this code. :)
>> What do you mean about miscompile?
>> I think this is safe because global-opt doesn't care about
>> !invariant.group metadata, which means that getting rid of
>> invariant.group.barrier will not break anything.
>>
>
> That's a good start, but why is it impossible to get a case where
> replacing the pointer is something globalopt does, then the pointer is used
> by a load in a function that globalopt didn't look at?
>
I am wating for test case as we disscuss
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150908/0325cdd2/attachment.html>