[PATCH] Remove seemingly dead InstCombine compare code

Eli Friedman eli.friedman at gmail.com
Fri Aug 2 20:05:15 PDT 2013


On Fri, Aug 2, 2013 at 6:27 PM, Matt Arsenault <arsenm2 at gmail.com> wrote:
>
> On Aug 2, 2013, at 16:47 , Eli Friedman <eli.friedman at gmail.com> wrote:
>
>> On Thu, Aug 1, 2013 at 1:34 PM, Matt Arsenault
>> <Matthew.Arsenault at amd.com> wrote:
>>> Turn check / insert cast into assertions that the sizes are the same. Add some more tests to try to break it. These paths seemed to not be tested with a datalayout. This checks if it needs to cast GEP indices, but at this point they should have already been converted to the pointer sized integer.
>>>
>>>
>>> http://llvm-reviews.chandlerc.com/D1261
>>>
>>> Files:
>>>  lib/Transforms/InstCombine/InstCombineCompares.cpp
>>>  test/Transforms/InstCombine/getelementptr.ll
>>>  test/Transforms/InstCombine/icmp.ll
>>>  test/Transforms/InstCombine/load-cmp.ll
>>
>> I'm pretty sure this patch is wrong.  instcombine will always
>> eventually transform GEPs in the way you expect, and in trivial cases,
>> we will visit the GEP before the icmp, but I think we can visit the
>> icmp first in more complicated cases.
>
> Do the test parts look OK? A number of places in that function aren't hit by any of the current tests. I'll keep trying to see if I can hit those places

test1_noinbounds looks strange... I'm pretty sure we shouldn't be able
to optimize it.

Some of the tests are missing CHECK lines.

-Eli




More information about the llvm-commits mailing list