[llvm-commits] [llvm] r127539 - /llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp

[Duncan Sands]

Hi Jin Gu Kang,

> I didn't know to add testcase to testsuite.
> Please Let me know how to add testcase.

testcases for instcombine go in test/Transforms/InstCombine.  There are
plenty of examples in there; I suggest you copy one that uses FileCheck.
Test that your testcase works by doing "make check" in the top-level
build directory.

>
> And I will insert assertions and then try to remove assumptions

OK, thanks.

Ciao, Duncan.

>
> Thanks your comment,
> Jin-Gu Kang
> ________________________________________
> From: Duncan Sands [baldrick at free.fr]
> Sent: Saturday, March 12, 2011 11:32 PM
> To: Jin Gu Kang
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] [llvm] r127539 -    /llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
>
> Hi Jin Gu,
>
>> I sended testcase on previous e-mail as following:
>
> it should be added to the testsuite.
>
>> and
>>
>>>     %A = alloca i32
>>>     %Ptr = getelementptr A, 0
>>>     %Ptr2 = bitcast i32 *%Ptr, i32*
>>>     %LI = load %Ptr2
>>>     do other stuff with Ptr2 that modifies the contents of %A
>>>     %ST = store %LI, %Ptr<-- would be wrong to remove the store
>>
>> "%Ptr2 = bitcast i32 *%Ptr, i32*" instruction is removed on InstCombine Pass
>> so equivalentAddressValues does not check this instrucion.
>
> I don't think you should rely on this.  I think you should rework the check so
> that it doesn't make any such "global" assumptions (i.e. assumptions about what
> code far away from this point does), only local assumptions (i.e. that can be
> verified by just reading the function itself).  An alternative is to add some
> assertions that check the assumptions you are relying on.
>
> Ciao, Duncan.
>
> PS: Please don't forget to add some descriptive comments.