[llvm-commits] [llvm] r59011 - in /llvm/trunk: include/llvm/Transforms/Utils/DbgInfoUtils.h lib/Transforms/Utils/DbgInfoUtils.cpp

[Chris Lattner]

On Nov 17, 2008, at 11:29 AM, Devang Patel wrote:
>>>> Instead of making this a "minipass" which is called by other  
>>>> passes, why not just make instcombine (and only instcombine) do  
>>>> this?  Instcombine runs many times in a typical compilation, and  
>>>> it can do this very cheaply (cost proportional to the number of  
>>>> debug intrinsics, instead of scanning entire functions even when  
>>>> there are none).
>>>
>>> However we want
>>> 	$ opt -adce foo.bc -o foo.optimzed.bc
>>> to preserve valid debug info.
>>
>> How wouldn't it?
>
> Well, this routine will be used by adce and any other pass that  
> removes dead instructions. If we just make instcombine responsible  
> for this cleanup then we are requiring that instcombine is always  
> scheduled after such passes. Is that OK ?

One of us is missing something here.  The change that this makes is an  
*optimization*, it isn't required for correctness, right?  If so,  
instcombine should be the only thing that does it.  ADCE doesn't do  
constant folding or many other possible cleanups, why would it do this  
one?

If this is required for correctness, then something else is seriously  
wrong.

>>>>>
>>>>> +    // If a llvm.dbg.stoppoint is placed just before an  
>>>>> unconditional
>>>>> +    // branch then remove the llvm.dbg.stoppoint intrinsic.
>>>>> +    else if (BranchInst *UC = dyn_cast<BranchInst>(Next)) {
>>>>> +      if (UC->isUnconditional()
>>>>
>>>> Why?  This doesn't seem right.
>>>
>>> If it is a unconditional branch then sooner or later the cfg will  
>>> be simplified and this may get in a way for a pass doing simple  
>>> clean.
>>
>> I don't understand.  You're saying that the intrinsic will prevent  
>> simplifycfg from doing a transformation?  That seems like a problem  
>> that should be fixed in the simplifycfg pass.
>
> Yes, simplifycfg pass should be fixed (if that is the case, I need  
> to verify it thoroughly). But many passes do their own little  
> cleanups. On the other side, unconditional branches are usually  
> removed by the optimizer, which means associated debug info is also  
> gone.

I can easily make LLVM IR that requires the stoppoint for valid debug  
info.  Something silly like this:

   stoppoint()
   br next

next:
   call bar()
   stoppoint()

You need the stoppoint before the branch to be able to stop on the  
call.  Zapping these unconditionally is not acceptable.

-Chris