[cfe-dev] clang::CodeGen::CodeGenModule::GetOrCreateLLVMGlobal

[Rafael Espíndola]

> I am not sure I understand the intended semantics of the UnnamedAddr argument to GetOrCreateLLVMGlobal.  Is it “this use does not care about the specific address”?  Because, if so, the (existing) logic is completely backwards: we need to be *clearing* the flag on the global if the argument is ever *false*.  And that seems like the most reasonable semantics, frankly.
>
> Also, the idea in the existing code that all runtime variables are unnamed_addr seems bogus to me.  __dso_handle is a specific example of a variable whose address value *is* semantically important.  This sort of thing would be important if there were any plausible transformations at all that involve unnamed_addr on external variable declarations.
>
> Also, the assertion is wrong: if something introduces a “runtime" declaration (e.g. by just declaring and using a global variable with the right name) before CodeGen emits its first intrinsic use of it, the declaration will not be marked unnamed_addr.
>
> So, basically, right now, this entire code path is useless and wrong.  Is there a plan to apply it in situations where it wouldn’t be useless, i.e. to non-runtime user declarations?

So, on how the patch is organized:

* The structure the patch uses is that when the variable is first
created, its unnamed_addr is set to the value of the argument.
* We then check that no user expect that a variable is unnamed_addr
when in fact it isn't. I tried to make the check more strict by
asserting that Entry->getUnnamedAddr() == UnnamedAddr, but that fails
deep down in some objc cases that create runtime variables and access
them as regular ones.

But to the more important point: Should we be adding unnamed_addr in
the first place? I am not sure. Back in
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20110110/037796.html
Chris asked if we should. I could not find the entire discussion (the
patch that ended up being committed is simpler) and I am still not
sure exactly when CreateRuntimeVariable should be used.

An alternative patch that simply never sets it in
CreateRuntimeVariable is attached.

Cheers,
Rafael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: t.patch
Type: application/octet-stream
Size: 2610 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140311/bfa3c196/attachment.obj>