[PATCH] D13740: Catch combine opportunities for redundant imuls

[Zia Ansari via llvm-commits]

zansari added a comment.

Thanks Elena and Sanjay for the review and comments.

- I liked the refactoring comment, and I made those changes. Looks much nicer now. Thanks.
- The code doesn't kick in for 64bit stuff, due to intermediate extends (coincidentally, given your recent patch :) ). That's why I didn't include 64bit in the lit test. I might be worth looking into this later to see if anything can be done.
- I beefed up the lit test, as you suggested, but I generalized it a little, since I'm not a fan of relying too much on register allocation and symbol table layout.
- I have no idea what happened to the lit test to duplicate the code. Sorry about that.




================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2221
@@ -2211,3 +2220,3 @@
     // Splat the sign bit into the register
     SDValue SGN =
         DAG.getNode(ISD::SRA, DL, VT, N0,
----------------
Will this replace both isConstantSplatVector and isa<Constan...> ? Or just the latter? I had a hard time trying to figure out the differences between the former, and isConstOrConstSplay to see if they can be both replaced.


http://reviews.llvm.org/D13740