[PATCH] D54640: [DAGCombiner] narrow truncated binops

Thu Nov 29 12:17:43 PST 2018

spatel added a comment.

In D54640#1313050 <https://reviews.llvm.org/D54640#1313050>, @andreadb wrote:

> LGTM.
>
> At the beginning, I was a bit concerned by the increase in the number partial register writes.
>  So I started experimenting with your patch fo a bit. In practice, I couldn't come up with an obvious case where your patch introduces extra partial register stalls. Also, none of the tests modified by your patch really suffers from partial register stalls.
>  The checks perfomed by your transform are quite conservative (the new combine rule is limited to pre-legalization, and it only affects binary opcodes where one of the operands is a constant). So, I am happy with it.

Thanks everyone for the reviews and comments!
I haven't thought about partial reg stalls in a while, but IIRC, AMD doesn't care in cases like these because it doesn't rename 8/16-bit regs, and Intel should be protected because we have a machine pass (-x86-fixup-bw-insts) to prevent the problem. Note that we promote ops to 32-bit (eg, D54803 <https://reviews.llvm.org/D54803>) for speed/size, so that reduces the chances for partial reg problems too. This was really just to fix that annoying rotate vs. shld/shrd bug. :)

I filed the 'movzwl' issue here:
https://bugs.llvm.org/show_bug.cgi?id=39840

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54640/new/

https://reviews.llvm.org/D54640