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

[Devang Patel]

On Nov 17, 2008, at 11:37 AM, Chris Lattner wrote:

> 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.

series of stoppoint()s with different line number values can confuse  
debuggers. I understand your point.

>>>>>>
>>>>>> +    // 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.


ok.
-
Devang