[PATCH] D14840: [X86] Detect SAD patterns and emit psadbw instructions on X86.

[Hal Finkel via llvm-commits]

hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.

In http://reviews.llvm.org/D14840#385786, @ab wrote:

> So, I understand the huge tests are required to trigger the reduction recognizer.  Have you considered changing that, for the sake of testability?  The most obvious thing I can think of is to represent the flag as metadata in IR. That has other benefits:
>
> - do the analysis in IR rather than in SelectionDAGBuilder, which is already a big enough mess as it is ;)


Doing this early means that we need to presuppose what the backend legalization will do, understand target features,etc. The fact that it is hard to test backend components should be addressed by improving those components, not by moving things into the IR level unnecessarily. There are a few places where we do this kind of thing out of necessity (because, for example, we lack sufficient loop analysis capabilities in the backend; PPCCTRLoops is an example), and it's a mess. You're certainly right, however, that SelectionDAGBuilder could use some refactoring (or at least partitioning). It might never happen, however, before the entire thing gets replaced by GlobalISel ;)

> - only do the analysis for targets that use it: if only X86 can select reduction ops, what's the point in recognizing them elsewhere?


Other targets will use this as well. PowerPC has vsumsws and friends. AArch64 has vaddv, etc.

> Another (possibly even crappier) alternative: add some "stress-test" commandline flag that overrides the isVectorReduction check on ADDs (or even adds the flag everywhere).


Honestly, the test cases don't look that bad. The IR, at least, is fairly succinct. It's never going to be super-small because we're matching a non-trivial pattern. The CHECK lines are definitely over constraining, but that's a symptom of the way they're autogenerated. I don't actually like tests like this which seem to strongly cover irrelevant register-alllocation decisions, but that's a larger problem with many tests in the backend. Hand-coded tests making proper use of CHECK-DAG and named regex patterns for allocated registers would be smaller.

LGTM.


http://reviews.llvm.org/D14840