[llvm-commits] [llvm] r128333 - /llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp

[Dan Gohman]

On Mar 26, 2011, at 5:42 PM, Bill Wendling wrote:

> On Mar 26, 2011, at 8:52 AM, Frits van Bommel wrote:
> 
>> On Sat, Mar 26, 2011 at 4:39 PM, Dan Gohman <gohman at apple.com> wrote:
>>> On Mar 26, 2011, at 2:32 AM, Bill Wendling wrote:
>>>> Simplification noticed by Frits.
>>>> 
>>>> Modified:
>>>>   llvm/trunk/lib/Transforms/Scalar/DeadStoreElimination.cpp
>> 
>>>> -  if ((EarlierOff == LaterOff && Earlier.Size <= Later.Size) ||
>>>> -      (EarlierOff > LaterOff &&
>>>> -       EarlierOff + Earlier.Size <= LaterOff + Later.Size))
>>>> +  if (EarlierOff >= LaterOff &&
>>>> +      EarlierOff + Earlier.Size <= LaterOff + Later.Size)
>>>>    return true;
>>> 
>>> I don't have time to fully investigate, but the testcase passes without the fix,
>> 
>> What testcase? This commit doesn't have a testcase (and doesn't need
>> one, given the next point).
>> 
>>> and the new code is now equivalent by De Morgan's law to the old code.
>> 
>> Hint: the commit message called it a "simplification" because it *is*
>> equivalent (but simpler) code.
>> 
>> I'm not sure where you're getting De Morgan's law from though, since
>> there's no negation anywhere...
>> (The only De Morgan's laws I know about are these:
>> <http://en.wikipedia.org/wiki/De_Morgan%27s_laws>)
> 
> I think he means from what it originally was. The reason I'm messing with this code is because of PR9561. What I did is a De Morgan's change from the old code.

Being equivalent means it doesn't fix the bug.

> But I disagree that the testcase passed before the previous change:
> 
> 	http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20110321/118574.html
> 
> At least it didn't pass for me. :)

Please check again :).

Dan