[llvm-commits] Please review: make bugpoint aware of blockaddress constant in global initializers

[Daniel Reynaud]

On Feb 7, 2012, at 5:44 PM, Eli Friedman wrote:

> On Tue, Feb 7, 2012 at 5:21 PM, Daniel Reynaud <dreynaud at apple.com> wrote:
>> Even better with the actual patch.
>> 
>> 
>> 
>> On Feb 7, 2012, at 5:20 PM, Daniel Reynaud wrote:
>> 
>> This is a patch for http://llvm.org/bugs/show_bug.cgi?id=11919
>> 
>> It seems to do the trick for me. When this particular configuration (a
>> global initializer references a blockaddress from a function that has been
>> sent to a different module) is not encountered, it should produce the same
>> initializer pattern as usual (i.e., everything in the safe module).
>> 
>> thanks,
>> daniel
> 
> +  bool globalInitUsesExternalBA(GlobalVariable* GV) {
> +      if (!GV->hasInitializer())
> +          return false;
> +
> +      Constant *I = GV->getInitializer();
> +
> +      // walk the values used by the initializer
> +      // (and recurse into things like ConstantExpr)
> +      std::vector<Constant*> Worklist;
> +      Worklist.push_back(I);
> +      while(!Worklist.empty()) {
> +        Constant* V = Worklist.back();
> +        Worklist.pop_back();
> +
> +        if (BlockAddress *BA = dyn_cast<BlockAddress>(V)) {
> +          Function *F = BA->getFunction();
> +          if (F->isDeclaration())
> +            return true;
> +        }
> +
> +        for (User::op_iterator i = V->op_begin(), e = V->op_end(); i != e; ++i)
> +          if (Constant *C = dyn_cast<Constant>(*i))
> +            Worklist.push_back(C);
> +      }
> +
> +    return false;
> +  }
> 
> You need to make sure not to recurse into GlobalValues.

ok

>  You also
> probably want a hashtable so you don't run into bad performance
> characteristics with ConstantExprs.

ok. I tried putting various things in the cache (everything, nothing, just ConstantExpr), and it did not have any noticeable impact on my test case (76s to reduce 459 functions).

> 
> Also, weird indentation.
> 
> 
> +  // Try to split the global initializers evenly
> +  for (Module::global_iterator I = M->global_begin(), E = M->global_end();
> +       I != E; ++I) {
> +    GlobalVariable *GV = cast<GlobalVariable>(NewVMap[I]);
> +    if (globalInitUsesExternalBA(I)) {
> +      assert(!globalInitUsesExternalBA(GV) && "A global initializer references"
> +          "functions in both the safe and the test module");
> 
> I don't see what prevents this assertion from triggering…

nothing indeed. If a global initializer references blockaddresses across modules, I don't think we can split.

Updated patch attached.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: bugpoint.patch
Type: application/octet-stream
Size: 2910 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120208/4ad0332e/attachment.obj>
-------------- next part --------------


-- daniel