[PATCH] D32006: Mark invariant.group.barrier as inaccessiblememonly

Piotr Padlewski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 8 15:27:47 PDT 2017


Prazek added a comment.

In https://reviews.llvm.org/D32006#748700, @hfinkel wrote:

> In https://reviews.llvm.org/D32006#748602, @Prazek wrote:
>
> > Is it possible to add "writeonly"? I am not sure if it will help in any way, but the tests seems to be working it.
>
>
> Yes, I think this makes sense. The model is that there's some side table holding the object type of all memory, and this barrier represents places where we might be updating that table to assign a different type. Right?


I am not sure about this because the barrier is inserted before changing the dynamic type, not after.
Even the Chandler's model was invalid in that way (the model was that the barrier returns pointer that aliases the argument, but
it has some special bits that represents the dyanmic type).

Maybe we can think about this as returning pointer aliasing the argument, but with some bits set in "unique" way, so that 2 barriers calls on the same pointer
returns different value?

I can give a brief summary of my thinking about what attributes we can have on the barrier and what can't
Argument:
+ readonly (alone): we can't read the argument because we won't be able to perform DSE

  store 42, %p ; dead store
  %b = barrier(%p)
  store 43, %b

+ writeonly (alone): we can't write through the argument because we won't be able to get values through the barrier:

  store 42, %p
  %b = barrier(%p)
  load %b

+ the only possible way is to have it as readnone (equivalent or stronger)

Function attributes (considering that the argument is readnone):
+ Can't be readonly alone, because it will be possible to remove barrier like:

  %b1 = barrier(%p)
  store 42, %b1, !invariant.group
  %b2 = barrier(%p) ; We could replace it with %b1 because  
  ; %p didn't escape, and %p aliases with %b1, so
  ; the barrier would read the same memory as %b1

I haven't checked that with the Capture tracking patch https://reviews.llvm.org/D32673
but if my thinking is correct then one day it could brake

+ Can't be writeonly alone, because if the barrier argument would escape before it,
we would consider barrier changing the value.

+ Can't be inaccessiblememonly & readonly, because we would be able to CSE 2 barriers like:

  %ptr2 = call i8* @llvm.invariant.group.barrier(i8* nonnull %ptr)
  store i8 43, i8* %ptr2, align 1, !invariant.group !0
  %ptr3 = call i8* @llvm.invariant.group.barrier(i8* nonnull %ptr)

Because the store would not change inaccessible memory

+ Can be marked as inaccessiblememonly & writeonly, but 
unfortunatelly we will loose the ability to CSE barriers like:

  %ptr2 = call i8* @llvm.invariant.group.barrier(i8* %ptr)
  %ptr3 = call i8* @llvm.invariant.group.barrier(i8* %ptr)

In https://reviews.llvm.org/D32006#748725, @sanjoy wrote:

> If that is the model then barrier can't be both `writeonly` *and* `argmemonly`, since the side table is disjoint from the object.


I am not sure if I follow, because I changed argmemonly to inaccessiblememonly


https://reviews.llvm.org/D32006





More information about the llvm-commits mailing list