Methods on addrspace pointers

I’ve been developing an optimization pass that uses the address space attribute (attribute((address_space(200))) to indicate different kinds of pointers to be treated differently (I’m making them be pointers to a “global” address space as in UPC).

However, I’ve hit a snag in the frontend code generator: if I try to make a method call on a pointer with a custom address space, the frontend chokes because it can’t bitcast between different address spaces, and methods are functions that take a pointer to “this” which is assumed to be in addrspace(0):

Assertion failed: (castIsValid(op, S, Ty) && “Invalid cast!”), function Create, file …/lib/IR/Instructions.cpp, line 2352.
0 clang-3.4 0x00000001039527f8 llvm::sys::PrintStackTrace(__sFILE*) + 40
1 clang-3.4 0x0000000103952c54 SignalHandler(int) + 388
2 libsystem_platform.dylib 0x00007fff802395aa _sigtramp + 26
3 libsystem_platform.dylib 000000000000000000 _sigtramp + 2145151600
4 clang-3.4 0x0000000103952ab6 abort + 22
5 clang-3.4 0x0000000103952a91 __assert_rtn + 81
6 clang-3.4 0x000000010385f8bf llvm::CastInst::Create(llvm::Instruction::CastOps, llvm::Value*, llvm::Type*, llvm::Twine const&, llvm::Instruction*) + 687
7 clang-3.4 0x00000001030a4d1c llvm::IRBuilder<true, llvm::ConstantFolder, llvm::IRBuilderDefaultInserter >::CreateCast(llvm::Instruction::CastOps, llvm::Value*, llvm::Type*, llvm::Twine const&) + 76
8 clang-3.4 0x0000000103bbeac3 clang::CodeGen::CodeGenFunction::EmitCall(clang::CodeGen::CGFunctionInfo const&, llvm::Value*, clang::CodeGen::ReturnValueSlot, clang::CodeGen::CallArgList const&, clang::Decl const*, llvm::Instruction**) + 3075
9 clang-3.4 0x0000000103c11c02 clang::CodeGen::CodeGenFunction::EmitCXXMemberCall(clang::CXXMethodDecl const*, clang::SourceLocation, llvm::Value*, clang::CodeGen::ReturnValueSlot, llvm::Value*, llvm::Value*, clang::QualType, clang::ConstExprIterator, clang::ConstExprIterator) + 738

I know it is not actually expected to be correct behavior to use other addrspace pointers in place of addrspace(0) pointers. My transformation passes change these calls, but I don’t get a chance to make the change because the frontend chokes first.

My questions are:

  1. Is there a more graceful way that these bitcasts could be handled? Code to load/store from alternate address spaces is allowed to be generated, it just segfaults when run. Can the same behavior be allowed without breaking things for method calls to these pointers? I can imagine simply adding code to detect if the address space is wrong and applying an addrspace cast to get around it.

  2. Is there a way to create methods that can be called with pointers to different address spaces? It seems there is no way to declare these in C++ using GNU attributes, and addrspace() seems to not be supported by C++11 attribute syntax, which could possibly express this.

Thanks,
-Brandon

Since 3.4 you need to use the addrspacecast instruction to cast between address spaces. It’s possible there are still some places left that haven’t been fixed yet to use it instead of creating bitcasts. You can use attribute((address_space(N))

Thank you for the prompt response, Matt.

Since 3.4 you need to use the addrspacecast instruction to cast between address spaces. It’s possible there are still some places left that haven’t been fixed yet to use it instead of creating bit casts.

Are you saying you think this is a bug and it should in fact be generating an addrspacecast? I will be looking into making this change, and if it would be accepted as a bug fix, I’d be happy to submit it.

  1. Is there a way to create methods that can be called with pointers to different address spaces? It seems there is no way to declare these in C++ using GNU attributes, and addrspace() seems to not be supported by C++11 attribute syntax, which could possibly express this.

You can use attribute((address_space(N))

I have already been using that syntax to annotate pointer declarations. However I don’t think it can be applied to methods the way I was thinking. At least it doesn’t actually change them:

struct Foo {
long x;

attribute((address_space(7))) void bar(long y) {
printf("%ld %ld\n", x, y);
}
};

Generates this declaration still:

define linkonce_odr void @_ZN3Foo3barEl(%struct.Foo* %this, i64 %y) ssp uwtable align 2

Though I think desired behavior would be:

define linkonce_odr void @_ZN3Foo3barEl(%struct.Foo addrspace(7)* %this, i64 %y) ssp uwtable align 2

(actually I think it’s more tricky because the address_space attribute could be applying to the return type. I’d think the correct way to specify the attribute on “this” would be to put it where “const” goes, after the parentheses)

From: "Brandon Holt" <bholt@cs.washington.edu>
To: "Matt Arsenault" <Matthew.Arsenault@amd.com>
Cc: "LLVM Developers Mailing List" <llvmdev@cs.uiuc.edu>
Sent: Monday, January 20, 2014 5:19:27 PM
Subject: Re: [LLVMdev] Methods on addrspace pointers

Thank you for the prompt response, Matt.

Since 3.4 you need to use the addrspacecast instruction to cast
between address spaces. It's possible there are still some places
left that haven't been fixed yet to use it instead of creating bit
casts.

Are you saying you think this is a bug and it should in fact be
generating an addrspacecast? I will be looking into making this
change, and if it would be accepted as a bug fix, I’d be happy to
submit it.

Looks like a bug (regardless, it certainly shouldn't crash like that).

2. Is there a way to create methods that can be called with pointers
to different address spaces? It seems there is no way to declare
these in C++ using GNU attributes, and addrspace() seems to not be
supported by C++11 attribute syntax, which could possibly express
this. You can use __attribute__((address_space(N))

I have already been using that syntax to annotate pointer
declarations. However I don’t think it can be applied to methods the
way I was thinking. At least it doesn’t actually change them:

struct Foo {
long x;

__attribute__((address_space(7))) void bar(long y) {
printf("%ld %ld\n", x, y);
}
};

Generates this declaration still:

define linkonce_odr void @_ZN3Foo3barEl(%struct.Foo* %this, i64 %y)
ssp uwtable align 2

Though I think desired behavior would be:

define linkonce_odr void @_ZN3Foo3barEl(%struct.Foo addrspace(7) *
%this, i64 %y) ssp uwtable align 2

(actually I think it’s more tricky because the address_space
attribute could be applying to the return type. I’d think the
correct way to specify the attribute on “this” would be to put it
where “const” goes, after the parentheses)

Does it make sense for 'this' to be in one address space on some functions and in a different address space in other functions? If so, should the address space of 'this' contribute to overload resolution?

-Hal

2. Is there a way to create methods that can be called with pointers
to different address spaces? It seems there is no way to declare
these in C++ using GNU attributes, and addrspace() seems to not be
supported by C++11 attribute syntax, which could possibly express
this. You can use __attribute__((address_space(N))

I have already been using that syntax to annotate pointer
declarations. However I don’t think it can be applied to methods the
way I was thinking. At least it doesn’t actually change them:

struct Foo {
long x;

__attribute__((address_space(7))) void bar(long y) {
printf("%ld %ld\n", x, y);
}
};

Generates this declaration still:

define linkonce_odr void @_ZN3Foo3barEl(%struct.Foo* %this, i64 %y)
ssp uwtable align 2

Though I think desired behavior would be:

define linkonce_odr void @_ZN3Foo3barEl(%struct.Foo addrspace(7) *
%this, i64 %y) ssp uwtable align 2

(actually I think it’s more tricky because the address_space
attribute could be applying to the return type. I’d think the
correct way to specify the attribute on “this” would be to put it
where “const” goes, after the parentheses)
Does it make sense for 'this' to be in one address space on some functions and in a different address space in other functions? If so, should the address space of 'this' contribute to overload resolution?

  -Hal

Yes it would. The struct itself has no specified address space, but the instances of it would.

__attribute__((address_space(1)) Foo f1;
__attribute__((address_space(2)) Foo f2;

f1.bar() and f2.bar() would need to call functions with different address spaced this pointers. Support for this would be required to support the OpenCL static C++ extension

Bringing this to the attention of the Clang list because that’s actually where my problems exist.

I’m working on using the “address_space” attribute to delineate special pointers in a pass I’m working on. This lets me intercept uses of these pointers and plug in calls to my runtime wherever there’s a load/store or other use. However, I’m having problems trying to call methods on pointers with this attribute.

I ran into a CodeGen problem where CodeGenFunction::EmitCall tried to bitcastan addrspace(N)* and failed an assertion. The attached patch is one possible fix, which is to use addrspacecast if the address spaces are different, otherwise just use bitcast as before. This has allowed me to then write my own pass to replace these instances with things from my runtime.

However, I have a new problem now: the Clang type checker seems to choke converting between __attribute__((address_space(N)) annotated types and const types, so that, though it allows me to call a non-const method on an addrspace pointer, it issues a compile error when I try to invoke a “const” method:

struct Foo {
void bar() const { /* … */ }
};

attribute((address_space(N)) Foo f;

void main() {
f.bar();
}

Results in the following error message:

error: cannot initialize object parameter of type ‘const Foo’ with an expression of type ‘attribute((address_space(200))) Foo’
f.bar(0);

It seems to me that const/non-const is orthogonal to address space attributes, so if one method invocation works, the other should as well. So my question is: which behavior is intended to be correct? Should Clang allow methods on arbitrary address spaces? (if not, how would one write a method to be used on the address space version?) It seems like this question must come up in the OpenCL/CUDA extensions, but I am not familiar with those so don’t know how this is handled there.

Thanks,
-Brandon

fix_addrspace_call.diff (1.24 KB)

I have since found many more places in Clang’s CodeGen that doesn’t take into account AddrSpaceCast when implicitly converting operands with BitCast. Is a better solution to instead just make BitCast check if the address spaces are different and apply an AddrSpaceCast instead? I made such a change in llvm/lib/IR/Instructions.cpp (see attached patch).

This seems like it misses the point of adding the AddrSpaceCast, and breaks assumptions because when you insert a BitCast you may or may not insert an AddrSpaceCast instead. Perhaps the correct solution is to find all the cases in Clang’s CodeGen where this may come up and ensure that we handle them all correctly…

implicit_addrspacecast.diff (1.1 KB)

fix_addrspace_call.diff (1.24 KB)

I’ve thought of trying that, but I think that will just hide many bugs. Most everywhere I’ve encountered that’s trying to bitcast between address spaces is a bug from using the default. Yes. I haven’t done as thorough of a search through the uses in clang as I have in core LLVM for this.