[PATCH] D54640: [DAGCombiner] narrow truncated binops

Craig Topper via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 29 10:13:20 PST 2018


craig.topper added inline comments.


================
Comment at: test/CodeGen/X86/clear-lowbits.ll:898-900
 ; X64-BMI2-NEXT:    movzwl %di, %eax
-; X64-BMI2-NEXT:    movl $16, %ecx
-; X64-BMI2-NEXT:    subl %esi, %ecx
+; X64-BMI2-NEXT:    movb $16, %cl
+; X64-BMI2-NEXT:    subb %sil, %cl
----------------
andreadb wrote:
> Not related to this patch: do you know why we need a movzwl here?
> 
> Only the lower 16-bits of EAX are 'defined' on exit from this function. Users of this function would not rely on the upper bits of EAX. So, - unless I am missing something - it is redundant to zero extend `%val`.
> 
I think it needs to be zero extended because we're using 32-bit shifts and you need zeros in the upper bits to guarantee that shifting right will shift 0s into the lower 16 bits.


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

https://reviews.llvm.org/D54640





More information about the llvm-commits mailing list