Localizing Globals ?

The code excerpt is from IPO/GlobalOpt.cpp

// If this is a first class global and has only one accessing function
    // and this function is main (which we know is not recursive we can make
    // this global a local variable) we replace the global with a local alloca
    // in this function.
    //
    // NOTE: It doesn't make sense to promote non single-value types since we
    // are just replacing static memory to stack memory.
    if (!GS.HasMultipleAccessingFunctions &&
        GS.AccessingFunction && !GS.HasNonInstructionUser &&
        GV->getType()->getElementType()->isSingleValueType() &&
        GS.AccessingFunction->getName() == "main" &&
        GS.AccessingFunction->hasExternalLinkage()) {
      DOUT << "LOCALIZING GLOBAL: " << *GV;

What if my global variable was into a different address space than stack?
How do I deny this optimization in that case? Can I find address space for stack from AllocaInst and then compare the address spaces for two?

- Sanjiv

What if my global variable was into a different address space than stack?

It doesn't matter in terms of semantics: because AnalyzeGlobal
returned false, we're guaranteed the address of the global is never
taken. I wouldn't be surprised if we end up generating invalid IR in
some cases, though, because of the semantics of replaceAllUsesWith.
Do you have a testcase that breaks?

How do I deny this optimization in that case? Can I find address space
for stack from AllocaInst and then compare the address spaces for two?

An alloca is guaranteed to be in address space zero, so you can just
check something like "GV->getType()->getAddressSpace() == 0".

-Eli

Eli Friedman wrote:

  

What if my global variable was into a different address space than stack?
    
It doesn't matter in terms of semantics: because AnalyzeGlobal
returned false, we're guaranteed the address of the global is never
taken. I wouldn't be surprised if we end up generating invalid IR in
some cases, though, because of the semantics of replaceAllUsesWith.
Do you have a testcase that breaks?

I do not really have a test case. I came across this when llvm-ld crashed while working with the debug information after such optimization.

How do I deny this optimization in that case? Can I find address space
for stack from AllocaInst and then compare the address spaces for two?
    
An alloca is guaranteed to be in address space zero, so you can just
check something like "GV->getType()->getAddressSpace() == 0".

-Eli
  

As you should, this check shouldn't be required semantically as the address is never taken, I would like to investigate more as to why llvm-ld crashed on the debug info.

- Sanjiv

Eli Friedman wrote:

  

What if my global variable was into a different address space than stack?
    
It doesn't matter in terms of semantics: because AnalyzeGlobal
returned false, we're guaranteed the address of the global is never
taken. I wouldn't be surprised if we end up generating invalid IR in
some cases, though, because of the semantics of replaceAllUsesWith.
Do you have a testcase that breaks?

The problem is replaceAllUsesWith asserts for type mismatch here. Try attached .bc with llvm-ld.

assert(New->getType() == getType() &&
         "replaceAllUses of value with new value of different type!");

Since stack is always on address space zero, I don't think that type of GV in a different address space is ever going to match.
The other way is to allow replaceAllUsesWith to ignore address spaces while comparing types. (do we have a way to do that ?).
But then such an optimization may fail the entire idea of user wanting to place a variable into different memory space. The original idea of user might be to save on the stack space (data memory) and hence he asked the variable to be placed into different memory space (program memory). So the best bet here is to deny this optimization by checking

GV->getType()->getAddressSpace() == 0.

- Sanjiv

test.bc (1.46 KB)

The problem is replaceAllUsesWith asserts for type mismatch here. Try
attached .bc with llvm-ld.

assert(New->getType() == getType() &&
"replaceAllUses of value with new value of different type!");

Ah, right; it probably isn't worth the effort to fix this
transformation to work with alternate address spaces.

But then such an optimization may fail the entire idea of user wanting to
place a variable into different memory space. The original idea of user
might be to save on the stack space (data memory) and hence he asked the
variable to be placed into different memory space (program memory). So the
best bet here is to deny this optimization by checking

GV->getType()->getAddressSpace() == 0.

Yeah, that sounds good; mind putting together a patch?

-Eli

But then such an optimization may fail the entire idea of user wanting to
place a variable into different memory space. The original idea of user
might be to save on the stack space (data memory) and hence he asked the
variable to be placed into different memory space (program memory). So the
best bet here is to deny this optimization by checking

GV->getType()->getAddressSpace() == 0.
    
Yeah, that sounds good; mind putting together a patch?

-Eli

Checked in.