[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