[PATCH] Promote GlobalMerge pass to a ModulePass to ease reuse

[Quentin Colombet]

Oh, that’s true the pass is not run where it was running.
Thanks Jim.

I’ll have to check if we can reproduce the old behavior since the pass may rely on running before the doInitialization of another pass.

Regarding the test cases, the debug information have changed.
MergedGlobals is no more referenced in the debug information. I’m fine with that since it was not in the program initially (we give insight on something the developer never wrote!), but it seems to be the expected behavior.

-Quentin

On Apr 16, 2013, at 5:31 PM, Jim Grosbach <grosbach at apple.com> wrote:

> Hi Quentin,
> 
> Hmmmm…. Perhaps we should consider whether to move the pass to where it was actually really running before. We’ll want to consider whether the old behavior was a bug, though, and it really belongs where it’s actually running now (later). A quick glance suggests that the test case may just need to be updated (i.e., the new order is fine, just different). We should understand a bit more of what’s going on before assuming that, though.
> 
> -Jim
> 
> On Apr 16, 2013, at 4:27 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> 
>> Hi,
>> 
>> I’ve noticed that all the work performed by the GlobalMerge pass is done within the doInitialization method.
>> It’s unclear to me if it is a valid design or not, because according to http://llvm.org/docs/WritingAnLLVMPass.html#the-functionpass-class:
>> - On one hand:
>> FunctionPass subclasses are not allowed to:
>> […]
>> 3. Add or remove global variables from the current Module.
>> - On the other hand:
>> The doInitialization method is allowed to do most of the things that FunctionPasses are not allowed to do.
>> 
>> What is clear however is: This design prevents other passes to take advantage of this pass. Let’s say I write a pass A that creates some global variables and I’d like to see these variables merged. Thus, I’ll add GlobalMerge pass after A. This does not work unless I push all the work of pass A into doInitialization…
>> 
>> The proposed patch turns GlobalMerge into a ModulePass and move all its work in runOnModule method.
>> Although this patch does not intend to make any functionality change it seems that the phase ordering implied by this design was expected!
>> 
>> Indeed, with this patch I have two make check failures related to debug info:
>> test/CodeGen/ARM/2011-01-19-MergedGlobalDbg.ll:26:14: error: expected string not found in input
>> ;CHECK-NEXT: .long __MergedGlobals
>>              ^
>> <stdin>:233:2: note: scanning from here
>>  .long _x2
>>  ^
>> --
>> and
>> test/CodeGen/ARM/2011-08-02-MergedGlobalDbg.ll:17:14: error: expected string not found in input
>> ;CHECK-NEXT: .long __MergedGlobals
>>              ^
>> <stdin>:205:2: note: scanning from here
>>  .long _x2
>>  ^
>> —
>> 
>> Any ideas on how I can fix those?
>> 
>> -Quentin
>> <GlobalMergeToModulePass.patch>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130416/14c838a4/attachment.html>