[PATCH] D6995: Reassociate: reprocess RedoInsts after each instruction

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 16 14:09:05 PST 2015


> On 2015-Nov-16, at 13:54, Mehdi Amini <mehdi.amini at apple.com> wrote:
> 
> 
>> On Nov 16, 2015, at 1:50 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>> 
>> 
>>> On 2015-Nov-16, at 13:47, Mehdi AMINI <mehdi.amini at apple.com> wrote:
>>> 
>>> joker.eph added a comment.
>>> 
>>> Only 3 test are still crashing now, but at the time I wrote the patch, I iterated on a fuzzer and these six tests (not eight) were stressing different patterns and different part of reassociate, so they're absolutely not redundant.
>> 
>> You could use six different functions in a single .ll file, right?
> 
> I “could"

I mean, that gives the same coverage, right?  Since it seems to reduce
the ongoing cost of running and maintaining the test(s), then merging
them seems better.

>>> I'm not sure either what CHECK line to put for a compiler crash non-regression, there are multiple cases in the test suite like this where FileCheck is not involved.
>> 
>> It's best practice to check for an expected output even if it's just
>> a crasher.  Most of the tests where FileCheck isn't involved are
>> legacy (and should be fixed if possible).
> 
> Should I just add a CHECK-LABEL with the function name? 
> What is the added value for the test?

Usually you can find *something* that adds value.  For this, I imagine
you could check that -reassociate transforms the IR the way you
expect...

> 
>> Mehdi
> 
> 
>> 
>>> 
>>> 
>>> http://reviews.llvm.org/D6995
>>> 
>>> 
>>> 
>> 
> 



More information about the llvm-commits mailing list