thr3ads.net - llvm dev - [LLVMdev] possible addrspacecast problem [Mar 2015]

If this information is useful, please help other people find it:
Share via:

Junjie Gu

2015-Mar-16 05:43 UTC

[LLVMdev] possible addrspacecast problem

Given a pointer,  does any addrspacecast affect the pointer's
dereferenceablity ?  For example,
    %pm = addrspaacecast float addrspacecast(n)* %pn to float
addrspacecast(m)*
    %r  = load float addrspace(m)* %pm
In another word. the question is whether the following is true ?
    isDereferenceablePointer(pn) == isDereferenceablePointer(pm)

  [Note that the function is defined as
   Value.cpp:isDereferenceaablePointer(const Value*, ...) ]


The reason I am asking this is that the current LLVM thinks the answer is
yes
(they are the same).  But for the following case, it will make
non-speculative
load to be moved out of its guard.  For example (assume that
isDereferenceablePointer(p)
is true),

The following code:

     %pm = addrspaacecast float* %p to float addrspacecast(m)*
     if (p is in addrspace(m))
        // BB_then
        %r0 = load float,  float addrspace(m)* %pm;
     else  // p is in default addrspace
        // BB_else
        %r1 = load float,  float* p;
     %r = phi float [r0, BB_then], [r1, BB_else]

will be transformed to

     %pm = addrspaacecast float* %p to float addrspacecast(m)*
     %r0 = load float addrspace(m)* %pm;
     %r1 = load float* p;
     %r = select i1 (p is in adrspace(m)) float %r0, float %r1

which is wrong as "%r0 = load float addrspace(m)* %pm" would be
illegal
if (p is in addrspace(m)) is false.

If we agree that the answer to the above question is no, then fix
is simple (fix diff is attached). I also have a small test to show
the problem,  simplifyCFG actually does the above transformation
(to reproduce, use "opt -simplifycfg -S test3.ll -o tt.ll).

Any suggestions/comments ?


thanks
Junjie
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150315/11514899/attachment.html>
-------------- next part --------------
Index: Value.cpp
==================================================================--- Value.cpp
(revision 232165)
+++ Value.cpp	(working copy)
@@ -574,8 +574,8 @@
       return isDereferenceablePointer(RelocateInst.derivedPtr(), DL, Visited);
     }
 
-  if (const AddrSpaceCastInst *ASC = dyn_cast<AddrSpaceCastInst>(V))
-    return isDereferenceablePointer(ASC->getOperand(0), DL, Visited);
+//  if (const AddrSpaceCastInst *ASC = dyn_cast<AddrSpaceCastInst>(V))
+//    return isDereferenceablePointer(ASC->getOperand(0), DL, Visited);
 
   // If we don't know, assume the worst.
   return false;

-------------- next part --------------
A non-text attachment was scrubbed...
Name: test3.ll
Type: application/octet-stream
Size: 1574 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150315/11514899/attachment.obj>

Sanjoy Das

2015-Mar-16 06:25 UTC

head link

[LLVMdev] possible addrspacecast problem

The LangRef says this about addrspacecast: "Note that if the address
space conversion is legal then both result and operand refer to the
same memory location.".  This brings forth two related questions:

 1. what happens if you execute an "invalid" addrspacecast?  Does this
    axiom make addrspacecast non-hoistable?  From a quick glance at
    the code, ValueTracking seems to assume that addrspacecasts can be
    speculatively executed.

 2. even if two pointers from two different address spaces point to
    the same "location", can one of them not be dereferenceable while
    the other is?  In other words, does dereferenceability depend on
    the addrspace of the pointer given that the "location" is
    dereferenceable?

On Sun, Mar 15, 2015 at 10:43 PM, Junjie Gu <jgu222 at gmail.com>
wrote:> Given a pointer,  does any addrspacecast affect the pointer's
> dereferenceablity ?  For example,
>     %pm = addrspaacecast float addrspacecast(n)* %pn to float
> addrspacecast(m)*
>     %r  = load float addrspace(m)* %pm
> In another word. the question is whether the following is true ?
>     isDereferenceablePointer(pn) == isDereferenceablePointer(pm)
>
>   [Note that the function is defined as
>    Value.cpp:isDereferenceaablePointer(const Value*, ...) ]
>
>
> The reason I am asking this is that the current LLVM thinks the answer is
> yes
> (they are the same).  But for the following case, it will make
> non-speculative
> load to be moved out of its guard.  For example (assume that
> isDereferenceablePointer(p)
> is true),
>
> The following code:
>
>      %pm = addrspaacecast float* %p to float addrspacecast(m)*
>      if (p is in addrspace(m))
>         // BB_then
>         %r0 = load float,  float addrspace(m)* %pm;
>      else  // p is in default addrspace
>         // BB_else
>         %r1 = load float,  float* p;
>      %r = phi float [r0, BB_then], [r1, BB_else]
>
> will be transformed to
>
>      %pm = addrspaacecast float* %p to float addrspacecast(m)*
>      %r0 = load float addrspace(m)* %pm;
>      %r1 = load float* p;
>      %r = select i1 (p is in adrspace(m)) float %r0, float %r1
>
> which is wrong as "%r0 = load float addrspace(m)* %pm" would be
illegal
> if (p is in addrspace(m)) is false.
>
> If we agree that the answer to the above question is no, then fix
> is simple (fix diff is attached). I also have a small test to show
> the problem,  simplifyCFG actually does the above transformation
> (to reproduce, use "opt -simplifycfg -S test3.ll -o tt.ll).
>
> Any suggestions/comments ?
>
>
> thanks
> Junjie
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>

Matt Arsenault

2015-Mar-16 23:07 UTC

head link

[LLVMdev] possible addrspacecast problem

On 03/15/2015 10:43 PM, Junjie Gu wrote:> Given a pointer,  does any addrspacecast affect the pointer's
> dereferenceablity ?  For example,
>     %pm = addrspaacecast float addrspacecast(n)* %pn to float 
> addrspacecast(m)*
>     %r  = load float addrspace(m)* %pm
> In another word. the question is whether the following is true ?
>     isDereferenceablePointer(pn) == isDereferenceablePointer(pm)
>
>   [Note that the function is defined as
>    Value.cpp:isDereferenceaablePointer(const Value*, ...) ]
>
>
> The reason I am asking this is that the current LLVM thinks the answer 
> is yes
> (they are the same).  But for the following case, it will make 
> non-speculative
> load to be moved out of its guard.  For example (assume that 
> isDereferenceablePointer(p)
> is true),
>
> The following code:
>
>      %pm = addrspaacecast float* %p to float addrspacecast(m)*
>      if (p is in addrspace(m))
>         // BB_then
>         %r0 = load float,  float addrspace(m)* %pm;
>      else  // p is in default addrspace
>         // BB_else
>         %r1 = load float,  float* p;
>      %r = phi float [r0, BB_then], [r1, BB_else]
>
> will be transformed to
>
>      %pm = addrspaacecast float* %p to float addrspacecast(m)*
>      %r0 = load float addrspace(m)* %pm;
>      %r1 = load float* p;
>      %r = select i1 (p is in adrspace(m)) float %r0, float %r1
>
> which is wrong as "%r0 = load float addrspace(m)* %pm" would be
illegal
> if (p is in addrspace(m)) is false.
>
> If we agree that the answer to the above question is no, then fix
> is simple (fix diff is attached). I also have a small test to show
> the problem,  simplifyCFG actually does the above transformation
> (to reproduce, use "opt -simplifycfg -S test3.ll -o tt.ll).
>
> Any suggestions/comments ?
>
>
> thanks
> Junjie

I think the pointer should still be dereferencable once casted, but 
since the testcase can be predicated on the address space your testcase 
looks broken. Have you tried making addrspacecast not 
isSafeToSpeculativelyExecute?

>
>
> fix.diff
>
>
> Index: Value.cpp
> ==================================================================> ---
Value.cpp	(revision 232165)
> +++ Value.cpp	(working copy)
> @@ -574,8 +574,8 @@
>         return isDereferenceablePointer(RelocateInst.derivedPtr(), DL,
Visited);
>       }
>   
> -  if (const AddrSpaceCastInst *ASC = dyn_cast<AddrSpaceCastInst>(V))
> -    return isDereferenceablePointer(ASC->getOperand(0), DL, Visited);
> +//  if (const AddrSpaceCastInst *ASC =
dyn_cast<AddrSpaceCastInst>(V))
> +//    return isDereferenceablePointer(ASC->getOperand(0), DL, Visited);
>   
>     // If we don't know, assume the worst.
>     return false;
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu          http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150316/2d1aabd5/attachment.html>

Junjie Gu

2015-Mar-17 04:13 UTC

head link

[LLVMdev] possible addrspacecast problem

pm = addrspacecast  float addrspace(n)* %pn  to float addrspace(m)*

If addrspace n and m refers to the same memory,  then
isDereferenceablePointter(pm) is the same as isDereferenceablePointer(pn).
This would be true for OpenCL on CPU. However,  if address space n and m
are disjoint (like in some GPU),  pn in address space n would be undefined
in address space m. Once addrspacecast'ed,  it would not be able to derive
dereferenceablity from the original pointer.  My original code segment
shows it as well.

My test case tries to have isDereferenceablePointer(pn) to be true (so
created it by alloca on stack,  which is always dereferenceable).  In this
way,  isDereferenceablePointer(pn) will be true as the current llvm
implemenation assumes isDeferenceablePointer(pn) isDereferenceablePointer(pn). 
The test case might be a not very good code,
 but it is correct.

"Have you tried making addrspacecast not
isSafeToSpeculativelyExecute?"
    isSafeToSpeculativelyExecute() invokes isDeferenceablePointer() to
check if it is speculative.  So, my patch will make
isSafeTospeculativeExecute() to return false.

"but since the testcase can be predicated on the address space your
testcase looks broken"
Could you elaborate on this ?




On Mon, Mar 16, 2015 at 4:07 PM, Matt Arsenault <Matthew.Arsenault at
amd.com>
wrote:
>  On 03/15/2015 10:43 PM, Junjie Gu wrote:
>
>   Given a pointer,  does any addrspacecast affect the pointer's
> dereferenceablity ?  For example,
>     %pm = addrspaacecast float addrspacecast(n)* %pn to float
> addrspacecast(m)*
>     %r  = load float addrspace(m)* %pm
> In another word. the question is whether the following is true ?
>     isDereferenceablePointer(pn) == isDereferenceablePointer(pm)
>
>   [Note that the function is defined as
>    Value.cpp:isDereferenceaablePointer(const Value*, ...) ]
>
>
> The reason I am asking this is that the current LLVM thinks the answer is
> yes
> (they are the same).  But for the following case, it will make
> non-speculative
> load to be moved out of its guard.  For example (assume that
> isDereferenceablePointer(p)
> is true),
>
> The following code:
>
>      %pm = addrspaacecast float* %p to float addrspacecast(m)*
>      if (p is in addrspace(m))
>         // BB_then
>         %r0 = load float,  float addrspace(m)* %pm;
>      else  // p is in default addrspace
>         // BB_else
>         %r1 = load float,  float* p;
>      %r = phi float [r0, BB_then], [r1, BB_else]
>
> will be transformed to
>
>      %pm = addrspaacecast float* %p to float addrspacecast(m)*
>      %r0 = load float addrspace(m)* %pm;
>      %r1 = load float* p;
>      %r = select i1 (p is in adrspace(m)) float %r0, float %r1
>
> which is wrong as "%r0 = load float addrspace(m)* %pm" would be
illegal
> if (p is in addrspace(m)) is false.
>
> If we agree that the answer to the above question is no, then fix
> is simple (fix diff is attached). I also have a small test to show
> the problem,  simplifyCFG actually does the above transformation
> (to reproduce, use "opt -simplifycfg -S test3.ll -o tt.ll).
>
>  Any suggestions/comments ?
>
>
>  thanks
>  Junjie
>
>
>
> I think the pointer should still be dereferencable once casted, but since
> the testcase can be predicated on the address space your testcase looks
> broken. Have you tried making addrspacecast not
> isSafeToSpeculativelyExecute?
>
>
>
>
> fix.diff
>
> Index: Value.cpp
> ==================================================================> ---
Value.cpp	(revision 232165)
> +++ Value.cpp	(working copy)
> @@ -574,8 +574,8 @@
>        return isDereferenceablePointer(RelocateInst.derivedPtr(), DL,
Visited);
>      }
>
> -  if (const AddrSpaceCastInst *ASC = dyn_cast<AddrSpaceCastInst>(V))
> -    return isDereferenceablePointer(ASC->getOperand(0), DL, Visited);
> +//  if (const AddrSpaceCastInst *ASC =
dyn_cast<AddrSpaceCastInst>(V))
> +//    return isDereferenceablePointer(ASC->getOperand(0), DL, Visited);
>
>    // If we don't know, assume the worst.
>    return false;
>
>
>
>
>  _______________________________________________
> LLVM Developers mailing listLLVMdev at cs.uiuc.edu        
http://llvm.cs.uiuc.eduhttp://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>
>-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150316/bb4ea16d/attachment.html>

David Chisnall

2015-Mar-17 08:41 UTC

head link

[LLVMdev] possible addrspacecast problem

On 16 Mar 2015, at 08:25, Sanjoy Das <sanjoy at playingwithpointers.com>
wrote:> 
> The LangRef says this about addrspacecast: "Note that if the address
> space conversion is legal then both result and operand refer to the
> same memory location.".  This brings forth two related questions:
> 
> 1. what happens if you execute an "invalid" addrspacecast?  Does
this
>    axiom make addrspacecast non-hoistable?  From a quick glance at
>    the code, ValueTracking seems to assume that addrspacecasts can be
>    speculatively executed.
> 
> 2. even if two pointers from two different address spaces point to
>    the same "location", can one of them not be dereferenceable
while
>    the other is?  In other words, does dereferenceability depend on
>    the addrspace of the pointer given that the "location" is
>    dereferenceable?
In our architecture, address space casts will never trap and are deterministic
(unless there is some asm with unmodelled side effects).  However, there are
pointers that can be dereferenced in one address space but not another:
speculatively performing the load would be a real bug.

Ideally, this would be a target-specific decision, but having the default be to
not propagate the dereferencing information and allowing targets to opt in.

I've just been debugging a related issue with regard to commutativity of
address space casts with respect to other operations.  One of the optimisers is
turning an add after an address space cast into an add before the address space
cast.  On our architecture, this results in different bounds information being
available on the pointer and a run-time crash.  We could represent casts with a
target-specific intrinsic, but we'd rather avoid that if possible. This is
not fixed by changing isSafeToSpeculativelyExecute to return false for
AddrSpaceCast, so I'll need to dig a bit more to understand exactly when and
why it happens.

I think there's a lot of code that still assumes that address space casts
are basically bitcasts.

David

Reasonably Related Threads

Search for more apparently analagous threads

llvm dev - Mar 2015 - [LLVMdev] possible addrspacecast problem

[LLVMdev] possible addrspacecast problem

[LLVMdev] possible addrspacecast problem

[LLVMdev] possible addrspacecast problem

[LLVMdev] possible addrspacecast problem

[LLVMdev] possible addrspacecast problem

Reasonably Related Threads