[PATCH] D33706: CodeGen: Cast temporary variable to proper address space

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: lib/CodeGen/CGCall.cpp:3805
+        Address Addr =
+            CreateMemTemp(I->Ty, ArgInfo.getIndirectAlign(), "tmp", false);
         IRCallArgs[FirstIRArg] = Addr.getPointer();
----------------
How about "indirect-arg-temp" as the name here?


================
Comment at: lib/CodeGen/CGCall.cpp:3835
+          Address AI =
+              CreateMemTemp(I->Ty, ArgInfo.getIndirectAlign(), "tmp", false);
           IRCallArgs[FirstIRArg] = AI.getPointer();
----------------
"byval-temp"


================
Comment at: lib/CodeGen/CodeGenFunction.h:1937
+  /// address space.
+
+  llvm::AllocaInst *CreateTempAlloca(llvm::Type *Ty, const Twine &Name = "tmp",
----------------
You have an extra newline here.

Hmm.  I see what you're getting at with your addition to the comment, but I think the
real problem is that my original recommendation is just wrong.  Let's fix that.

"LangAS::Default is the address space of pointers to local variables and temporaries,
as exposed in the source language.  In certain configurations, this is not the same as
the alloca address space, and a cast is needed to lift the pointer from the alloca AS
into LangAS::Default.  This can happen when the target uses a restricted address space
for the stack but the source language requires LangAS::Default to be a generic address
space.  The latter condition is common for most programming languages; OpenCL is an
exception in that LangAS::Default is the private address space, which naturally maps
to the stack.

Because the address of a temporary is often exposed to the program in various ways,
this function will perform the cast by default.  The cast may be avoided by passing false
as \p CastToDefaultAddrSpace; this is more efficient if the caller knows that the address
will not be exposed."

And you should rename the parameter to CastToDefaultAddrSpace here and the other
places.


https://reviews.llvm.org/D33706