[llvm-commits] [llvm] r127834 - in /llvm/trunk: include/llvm/Transforms/Utils/Local.h lib/Transforms/InstCombine/InstructionCombining.cpp lib/Transforms/Utils/Local.cpp test/Transforms/InstCombine/debuginfo.ll

[Evan Cheng]

Hi Devang,

I agree with Eli. This seems like an expensive and potentially unsafe solution.

Evan

On Mar 17, 2011, at 5:40 PM, Eli Friedman wrote:

> On Thu, Mar 17, 2011 at 3:41 PM, Devang Patel <dpatel at apple.com> wrote:
>> 
>> On Mar 17, 2011, at 3:36 PM, Eli Friedman wrote:
>> 
>>> On Thu, Mar 17, 2011 at 3:18 PM, Devang Patel <dpatel at apple.com> wrote:
>>>> Author: dpatel
>>>> Date: Thu Mar 17 17:18:16 2011
>>>> New Revision: 127834
>>>> 
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=127834&view=rev
>>>> Log:
>>>> Try to not lose variable's debug info during instcombine.
>>>> This is done by lowering dbg.declare intrinsic into dbg.value intrinsic.
>>>> Radar 9143931.
>>> 
>>> This looks like the wrong solution to a problem only caused by a few
>>> very specific pieces of instcombine.  Adding an extra iteration over
>>> every instruction in the function isn't cheap
>> 
>> instcombine is already going to iterator over once. If there are compile time regressions because of this patch, I'll take a look asap.
> 
> Not sure there will be; worth watch for, though.
> 
>>> , and it looks like this
>>> could do some serious damage to the debug info for cases other than
>>> the given testcase.
>> 
>> ?
> 
> Assuming I'm reading the change correctly, take an example like the following:
> void foo(int* x) { *x = 10; }
> int bar(int i) {
>  int a;
>  a = i;
>  foo(&a);
>  return a; // If you "print a" at this point, you'll get the value of
> i, not 10.
> }
> 
> -Eli
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits