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

[John McCall]

On Mar 10, 2014, at 8:40 AM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
> bccing cfe-dev, moving the cfe-commits
> 
> On 10 March 2014 09:58, Vassil Vassilev <vasil.georgiev.vasilev at cern.ch> wrote:
>> Hi,
>>  I noticed that CodeGenModule.cpp (around line number 1514):
>> 
>>    if (UnnamedAddr)
>>      Entry->setUnnamedAddr(true);
>> 
>> is executed only in the 'Get'-branch. I.e if the global doesn't exist this
>> flag is not taken into account. Is that an inconsistency or a desired
>> behavior?
> 
> That is a bug, thanks!
> 
> For
> 
> struct Foo {
>  ~Foo();
> };
> Foo x;
> 
> We will output "@__dso_handle = external global i8", but an
> unnamed_addr gets added if a second "Foo y" is added.
> 
> John (or Fariborz), can you check if the attached patch is doing the
> correct thing for obj-c?

The change to the test case is fine: the program does not rely on the address value of _NSConstantStringClassReference.  I guess.

But.

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?

John.