[llvm-commits] Improvements to PassManager

[Nick Lewycky]

Pedro Artigas wrote:
> Hello evan,
>
> I believe that there should be no ordering assumption between any doFinalization/doInitialization calls, that is what got us in trouble in the first place. The current assumption is that doFinalization of AddressSanitizer is run before runOnModule of the printer which is, AFAIK, an unreasonable assumption and a current implementation detail.

Hi Pedro, I think I'm responsible for the way that ASAN uses 
doInitialization/doFinalization, by suggesting it in review when the 
pass was added.

My understanding is that function passes are expected to do things to 
the module in their init/finalize functions that they can't do in 
runOnFunction -- in this case, creating and then cleaning up the 
function declarations. Trying to do that in runOnFunction would break 
parallel runs of function passes.

I didn't think of what would happen if you put the asm printer in the 
same passmanager. You're right that it doesn't work, but it's not 
immediately obvious to me that this is asan's fault, or the user's fault 
for putting a per-function asm printer inside the same pass manager.

Nick

> Thanks
>
> Pedro
>
>
> On Nov 16, 2012, at 4:02 PM, Evan Cheng<evan.cheng at apple.com>  wrote:
>
>> Hi Pedro,
>>
>> Question about changes to MPPassManager::runOnModule().
>>
>> +  // Initialize module passes
>> +  for (unsigned Index = 0; Index<  getNumContainedPasses(); ++Index)
>> +    Changed |= getContainedPass(Index)->doInitialization(M);
>> +
>>    for (unsigned Index = 0; Index<  getNumContainedPasses(); ++Index) {
>>      ModulePass *MP = getContainedPass(Index);
>>      bool LocalChanged = false;
>> @@ -1600,6 +1606,13 @@
>>      removeDeadPasses(MP, M.getModuleIdentifier(), ON_MODULE_MSG);
>>    }
>>
>> +  // FIX_ME: Would like to really do it but it breaks AddressSanitizer
>> +  // due do embedded assumption there
>> +  //
>> +  // Finalize module passes
>> +  //for (unsigned Index = 0; Index<  getNumContainedPasses(); ++Index)
>> +  //  Changed |= getContainedPass(Index)->doFinalization(M);
>> +
>>    // Finalize on-the-fly passes
>>    for (std::map<Pass *, FunctionPassManagerImpl *>::iterator
>>         I = OnTheFlyManagers.begin(), E = OnTheFlyManagers.end();
>>
>> Why not initialize module passes before initialize on-the-fly passes and finalize them after on-the-fly passes are finalized? Wouldn't that fix the ASAN issue?
>>
>> Evan
>>
>> On Nov 16, 2012, at 2:49 PM, Pedro Artigas<partigas at apple.com>  wrote:
>>
>>> Hello All,
>>>
>>> The Patch below incorporates some comments from my prior submit, namely, no need for extra doInitialization/doFinalization in the PassManager interface. It also adds code to call doInitialization in all passes in the runOnModule method of MPPassManager. The only remaining issue is the doFinalization that cannot be called at the same spot as doInitialization because it breaks an assumption made by AddressSanitizer.
>>>
>>> It also does not incorporate the suggestion of moving the doInitialization/doFinalization methods to the Pass class. That suggestion is on the pipeline and should be addressed by a later commit.
>>>
>>> Thanks
>>>
>>> Pedro Artigas
>>>
>>> <patch.txt>_______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>