yaxunl added inline comments.
================ Comment at: lib/CodeGen/CGDecl.cpp:1119 + T.getAddressSpace(), address.getPointer()->getType()-> + getPointerElementType()->getPointerTo(getContext().getTargetAddressSpace( + T.getAddressSpace())), true); ---------------- rjmccall wrote: > Hmm. I would prefer if we could avoid doing this in the fast path. > Suggestion: > > - Please add a variable (ASTAllocaAddressSpace) to CodeGenTypeCache that > stores the AST address space of the stack. You can add a function to > CodeGenTargetInfo to initialize this; in most targets, it will just return 0, > but your target can make it ~0U or something, anything works as long as it > doesn't incorrectly match an actual target address space. (But you should > really consider adding an actual target-specific address space for the stack! > It'll probably be useful in builtin code or something eventually.) > > - Please add an assert that T.getAddressSpace() == 0, and then make this > call conditional on whether ASTAllocaAddressSpace is non-zero. > > You can just write address.getElementType() instead of > address.getPointer()->getType()->getPointerElementType(). > > Are you intentionally leaving emission.Addr as the unconverted pointer? That > will probably mess everything else downstream that uses 'emission', and the > only reason you're not seeing that is that OpenCL probably doesn't actually > have anything downstream that uses 'emission' — destructors or whatever else. > > Please add an inline comment like > , /*non-null*/ true); Sorry I missed this comment. I will do the changes. About emission.Addr, I leave the unconverted pointer intentionally. I need this to work for both C++ and OpenCL. I checked the usage of emission.Addr and they seem to be only load or store, so I think it is safe to use the unconverted pointer. Now you mention destructor. I think I need to change that and I am going to add a lit test also. https://reviews.llvm.org/D32248 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
