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

[Hal Finkel]

----- Original Message -----
> From: "Chandler Carruth" <chandlerc at google.com>
> To: "Adam Nemet" <anemet at apple.com>
> Cc: "llvm commits" <llvm-commits at cs.uiuc.edu>
> Sent: Monday, April 28, 2014 7:19:02 PM
> Subject: Re: [PATCH][LoopStrengthReduce] Don't trim a formula that uses a	subset of required registers
> 
> 
> 
> 
> 
> 
> On Mon, Apr 28, 2014 at 5:12 PM, Adam Nemet < anemet at apple.com >
> wrote:
> 
> 
> 
> OK, so as I understand this is the plan:
> 
> 
> * file a PR on the original LSR problem with the C test case
> * check in the fix again (without the test case), referencing the PR
> * keep the PR open at this point until we figure out a way to have an
> LLVM IR test case for this
> 
> 
> I would file the PR with the C test case for "llc cannot reproduce
> LSR behavior observed with clang" or some such. It should have
> nothing to do with the particular bug fix, and the bug fix shouldn't
> reference the PR (we don't ever reference PRs from source code).
> 
> 
> Another way to look at this is that the PR should essentially be
> "make llc able to test this, and commit the test case when that
> works".
> 
> 
> 
> * remove clang from the substitutions in lit.cfg so that the next
> person won’t get confused like I got
> Sure, but in a separate commit.
> 
> 
> 
> 
> Despite all of this, I'm a little worried that this will get shelved
> in a PR and no one will ever look at it. This kind of deficiency in
> our testing infrastructure seems like a really scary and important
> thing, but its not even something I know how to chase down (not
> being an expert on LSR).

I agree, let's commit the fix and then worry about the regression test.

That having been said, trying to produce a test case, for either Clang or llc, that tries to reproduce a specific use-def-chain ordering strikes me as extremely fragile. I know of no requirement that use-def ordering be stable across platforms, because, for example, the order of instruction insertion can depend on a pointer-set iteration order (so long as, in the end, the IR itself is deterministic).

I'm not saying this to imply that understanding the Clang vs. llc difference is not important, because it is important. However, if we want to produce a useful regression test for this, we might need to 'bake' everything using a C++-level unit test. In my experience, tests that depend on a specific use-def ordering (I've run into a number of these at the SDAG level over the years) silently stop actually testing anything at some point due to some unrelated change (or they only ever tested anything with some particular malloc implementation on some particular platform).

 -Hal

> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory