[LLVMdev] addrspace attribute and intrisics

[Benedict Gaster]

Hi,

Great I look forward to the patch!

One comment I had was on the name of the overloaded intrinsics. In  
your example you resolve the atomic.load.add to:

	@llvm.atomic.load.add.p0i32     // i32 ptr to default address space
	@llvm.atomic.load.add.p11i32   // i32 ptr to address space 11

and I was wondering could they instead be named:

	@llvm.atomic.load.add.p0.i32     // i32 ptr to default address space
	@llvm.atomic.load.add.p11.i32   // i32 ptr to address space 11

For me this is a little easier to read and separates out the type  
components.  I expect that there might be an LLVM naming convention  
that I am not aware off, that invalidates my suggestion.

Given these changes enable atomics with the ability to work on  
different address spaces, there becomes the need to be able to express  
barriers for these different address spaces. The problem is that the  
current barrier operation does not provide enough granularity:
	declare void @llvm.memory.barrier( i1 <ll>, i1 <ls>, i1 <sl>, i1  
<ss>, i1 <device> )
I was wondering if this would be possible to fix?  One possible  
solution to this is to provide an additional argument which ranges  
over valid address spaces, e.g.:
	declare void @llvm.memory.barrier( i1 <ll>, i1 <ls>, i1 <sl>, i1  
<ss>, i32 addrspace, i1 <device> )
and we can now write the code:
	@llvm.atomic.load.add.p0i32
	@llvm.atomic.load.add.p11i32

         %r1 = call i32 @llvm.atomic.load.add.p0i32( i32 addrspace(0)*  
%ptr0, i32 4)
	%r2 = call i32 @llvm.atomic.load.add.p11i32( i32 addrspace(11)*  
%ptr11, i32 4)
	call void @llvm.memory.barrier(i1 true, i1 true, i1 false, i1 false,  
i32 11, i1 false) ; force read-modify-write %ptr11 to complete

A problem with this approach is that developing a new pass over the IL  
that works with address spaces, will have to include knowledge that  
for certain operations address space information is encoded as data  
rather than as types and in general, it may not be able to statically  
determine the particular address space. An alternative is to  
generalize the barrier operation to a more general memory fence, that  
as a special case reduces to the above barrier, e.g.:
	declare void @llvm.memory.fence( i1 <ll>, i1 <ls>, i1 <sl>, i1 <ss>,  
i32 addrspace(11)*<ptr>, i1 <device>, i1 barrier )
The code above then becomes:
	@llvm.atomic.load.add.p0i32
	@llvm.atomic.load.add.p11i32

         %r1 = call i32 @llvm.atomic.load.add.p0i32( i32 addrspace(0)*  
%ptr0, i32 4)
	%r2 = call i32 @llvm.atomic.load.add.p11i32( i32 addrspace(11)*  
%ptr11, i32 4)
	call void @llvm.memory.fence(i1 true, i1 true, i1 false, i1 false,  
i32 addrspace(11)* %ptr11, i1 false, i1 true)

If a particular target supports fence operations with explicit  
addresses the last line could be replaced with the more fine grained:

	call void @llvm.memory.fence(i1 true, i1 true, i1 false, i1 false,  
i32 addrspace(11)* %ptr11, i1 false, i1 false)
Does this make sense or have I missed something that would enable the  
existing llvm.memory.barrier operation to be used in the context of  
address spaces?

Ben

On 5 Jul 2008, at 21:33, Mon P Wang wrote:

> Hi,
>
> I got pulled off doing other things last week but I plan to get the
> support for address spaces to the intrinsics this week. As Benedict
> noted, the problem is that we don't carry the address space
> information with the intrinsics.  Today, we will do an implicit cast
> to the default address space.  My change will prevent that from
> happening by allowing the intrinsic to have a signature with an
> address qualified pointer.   This will change the name of the
> intrinsics.  When I talked to Dan about this, we thought it made sense
> to add a pointer qualifier to the name.  For example, the name of the
> atomic add will change from @llvm.atomic.load.add.i32 to
>    @llvm.atomic.load.add.p0i32     // i32 ptr to default address space
>    @llvm.atomic.load.add.p11i32   // i32 ptr to address space 11
>
> This means we will auto convert the old names to the new one.   I'm
> also planning to change clang to generate an error if the compiler
> implicitly cast a pointer between two different address spaces.  If
> anyone has an issue or concerns, please let me know.
>
>   -- Mon Ping
>
>
> On Jul 3, 2008, at 10:11 AM, Chris Lattner wrote:
>
>> On Thu, 3 Jul 2008, Benedict Gaster wrote:
>>> I am slightly unclear about the semantics of the addrspace
>>> attribute and there use with intrinsics. For example, is the
>>> following code valid:
>>>
>>> % ptr      = malloc i32 addrspace(11)
>>> % result = call i32 @llvm.atomic.load.add.i32( i32 addrspace(11)*
>>> %ptr, i32 4);
>>>
>>> If this is valid it means that a certain amount of type information
>>> is lost at the LLVM IL level and if it is not valid, then it is
>>> impossible to use any of the built-in functions for anything other
>>> than the default address space.
>>
>> That is not currently supported, but Mon Ping is working on it (as I
>> understand).  Mon Ping, are you still working on this?  If so, what
>> is the status?
>>
>> -Chris
>>
>> -- 
>> http://nondot.org/sabre/
>> http://llvm.org/
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>