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

[Jin Gu Kang]

Hi Duncan

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

And I will insert assertions and then try to remove assumptions

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.