[PATCH] D28200: [BypassSlowDivision] Do not bypass division of hash-like values

Fri Mar 24 14:14:32 PDT 2017

n.bozhenov added a comment.

In https://reviews.llvm.org/D28200#692591, @jlebar wrote:

> > I believe that the new version of the heuristic is much more robust than the previous one. However, a naive implementation didn't work initially. It turned out that hash values weren't recognized because of PHI nodes with undef incoming values generated by loop-unroll. I had to amend the patch to work around the problem.
>
> It doesn't look like you added any "unit" tests to cover this changed behavior.  Was that intentional?

I slightly updated a few tests for a newer heuristic, but generally the tests stay the same. It was intentional, yes.

> Ultimately I'm not comfortable LGTM'ing this patch so long as it contains this -O3 test.  This is very different than how I understood we write tests.  But that might just be my ignorance speaking; what you're trying to do may in fact be perfectly fine; I really don't know enough to say for sure.  Let's get a second opinion?  If @hfinkel or someone thinks this is the right approach, I'm happy.

Ok. I have removed the test in question. Probably I kind of abused the lit testing infrastructure.

https://reviews.llvm.org/D28200