[llvm-commits] [llvm] r130485 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombineShifts.cpp test/Transforms/InstCombine/shift.ll

[Devang Patel]

On May 1, 2011, at 12:40 PM, Chris Lattner wrote:

> On Apr 30, 2011, at 3:22 AM, Benjamin Kramer wrote:
> 
>> 
>> On 30.04.2011, at 00:00, Devang Patel wrote:
>> 
>>> 
>>> On Apr 29, 2011, at 2:50 PM, Benjamin Kramer wrote:
>>>> 
>>>> Are you sure this specific transformation destroys the DebugLoc? When InstCombine simply replaces one
>>>> instruction with another it preserves debug information (InstructionCombining.cpp:1592).
>>> 
>>> I added line 1592 however I am not sure that this particular transformation is covered by 1592 in all cases. I have seen "shl" without DebugLoc generated by instcombine before your patch landed in svn. If you're interested I can send you more info.
>> 
>> There is a way this transform can lose a DebugLoc
>> 
>> x = C1 <<  A # with DebugLoc
>> y = x  << C2 # without DebugLoc
>> 
>> InstCombine will then replace 'y' with a new instruction (but there's no DebugLoc to attach to it) and 'x' is DCE'd.
>> I think the only feasible way to improve here is to find out why 'y' has no debug info attached to it.
>> 
>> If you have a test case where InstCombine introduces SHLs without DebugLocs, I'd be interested in seeing it.
> 
> More generally, we don't want to sprinkle setDebugLoc calls into *every* instcombine xform.  

One other alternative is to consistently use IRBuilder.

> We need a better solution.

I agree.
-
Devang