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

Piotr Padlewski via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 8 13:58:30 PDT 2015


On Tue, Sep 8, 2015 at 1:42 PM, Nick Lewycky <nlewycky at google.com> wrote:

>
>
> On 8 September 2015 at 12:41, Piotr Padlewski <prazek at google.com> wrote:
>
>>
>>
>> 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
>>
>
> OK.
>
> ================
>>>> 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
>>
>
> @s1 = unnamed_addr internal global i8*
> @s2 = unnamed_addr internal global i8*
>
> define void @__global_var_init() {
>   %p1 = ...
>    %p2 = call i8* @invariant.group.barrier(i8* %p1)
>   store i8* %p1, i8** @s1
>   store i8* %p2, i8** @s2
>   ret void
> }
>
> define void @runtime_function() {
>   %p1 = load i8*, i8** @s1
>   %p2 = load i8*, i8** @s2
>   %A = load i8* %p1, !invariant.group A
>   store ...
>   %B = load i8* %p2, !invariant.group A
>   ...
> }
>
> The order of optimization is:
>   1. globalopt turns %p2 into %p1 internally
>   2. it puts %p1 into @s1 and @s2
>   3. globalopt evaluation succeeds, it commits %p1 to both @s1 and @s2
>   4. there is no need for @s1 and @s2, they are unnamed_addr and always
> contain the same value, they are folded into one (@s2->RAUW(@s1))
>   5. therefore %p1 and %p2 are combined (%p2->RAUW(%p1))
>   6. now both %A and %B are loads with the same SSA variable and
> invariant.group, but they used to have an invariant.group.barrier between
> them
>
> Nick
>
> Ok, but in this case we load the same value so having and not having
invariant.group.barrier between them doesn't change anything. I could be
wrong if load of %A and %B would be different, but we can't get case like
this.
Second thing, is that I don't think we would be able to perform non local
optimizations - if __global_bar_init() would be a normal function, then
when we would ask about memory dependencie of %B or %A, we would get
clobber,
because we would run on the beginning of function. Am I right?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150908/1947a5d0/attachment-0001.html>


More information about the llvm-commits mailing list