[PATCH][LoopStrengthReduce] Don't trim a formula that uses a subset of required registers

[Andrew Trick]

On Apr 28, 2014, at 3:55 PM, Chandler Carruth <chandlerc at google.com> wrote:

> 
> On Fri, Apr 25, 2014 at 1:39 PM, Andrew Trick <atrick at apple.com> wrote:
> Adam,
> 
> This fix looks great to me! Please go ahead and commit.
> 
> Andy, how does this look good? ;]

The fix really does looks great, I reviewed it quite carefully ;)

I was actually impressed that Adam figured out how to test it, and that lit was somehow smart enough not to run a test when clang wasn’t built. When I reviewed it, I just did this:

$ find . -name \*.cfg | xargs grep suffix

and saw a bunch of other LLVM dirs recognizing C code. But I suppose someone cleaned up the actual C code without cleaning up the dir config.

Anyway, this bug has been around for a year and the one of the major difficulties in getting it fixed was we just have absolutely no way to force it in llc. Once you serialize the IR, the problem goes away.

I recommend checking the fix back in, filing a PR for the bug, referencing the PR from the commit, and attaching a nice C test case to the bug. LSR is nearly impossible to unit test when the bug requires a large set of formulae.

-Andy

> 
> --- /dev/null
> +++ b/test/Transforms/LoopStrengthReduce/ARM64/req-regs.c
> @@ -0,0 +1,36 @@
> +// RUN: clang %s -O3 -target arm64-apple-ios -o - -S -mllvm -debug-only=loop-reduce 2>&1| FileCheck %s
> +// REQUIRES: asserts
> +
> +// LSR used to fail here due to a bug in the ReqRegs test.  To complicate
> +// things, this could only be reproduced with clang because the uses would
> +// come out in different order when invoked through llc.
> +
> +// CHECK: The chosen solution requires
> +// CHECK-NOT: No Satisfactory Solution
> 
> This test runs clang, from the users PATH, with -O3, in an LLVM test. This doesn't work *at all*.
> 
> That really doesn't work. Mind reverting until you have a reduced test case? This is blocking us, so I'd like to get it fixed ASAP.