This is an archive of the discontinued LLVM Phabricator instance.

[X86] Improve handling of UDIVREM8_ZEXT_HREG/SDIVREM8_SEXT_HREG to support 64-bit extensions.
ClosedPublic

Authored by craig.topper on Sep 26 2017, 12:21 AM.

Details

Summary

The code for handling UDIVREM8_ZEXT_HREG with a 64-bit result type has been dead since r256924.

Though I'm not entirely sure it should be dead. The output from the test case from that revision shows an extra movzx that we could get rid of if this code was live. So I'm posting this patch to start a conversation

define i64 @pr25754(i8 %a, i8 %c) {
; X32-LABEL: pr25754:
; X32:       # BB#0:
; X32-NEXT:    movzbl {{[0-9]+}}(%esp), %eax
; X32-NEXT:    # kill: %EAX<def> %EAX<kill> %AX<def>
; X32-NEXT:    divb {{[0-9]+}}(%esp)
; X32-NEXT:    movzbl %ah, %ecx
; X32-NEXT:    movzbl %al, %eax
; X32-NEXT:    addl %ecx, %eax
; X32-NEXT:    xorl %edx, %edx
; X32-NEXT:    retl
;
; X64-LABEL: pr25754:
; X64:       # BB#0:
; X64-NEXT:    movzbl %dil, %eax
; X64-NEXT:    # kill: %EAX<def> %EAX<kill> %AX<def>
; X64-NEXT:    divb %sil
; X64-NEXT:    movzbl %ah, %ecx
; X64-NEXT:    movzbl %cl, %ecx
; X64-NEXT:    movzbl %al, %eax
; X64-NEXT:    addq %rcx, %rax
; X64-NEXT:    retq
  %r1 = urem i8 %a, %c
  %d1 = udiv i8 %a, %c
  %r2 = zext i8 %r1 to i64
  %d2 = zext i8 %d1 to i64
  %ret = add i64 %r2, %d2
  ret i64 %ret
}

Diff Detail

Event Timeline

craig.topper created this revision.Sep 26 2017, 12:21 AM
RKSimon edited edge metadata.Oct 1 2017, 5:46 AM

Is this a missing simpifydemandedbits pattern?

craig.topper retitled this revision from [X86] Remove dead code from X86DAGToDAGISel's handling of DIV to [X86] Imiprove handling of UDIVREM8_ZEXT_HREG/UDIVREM8_SEXT_HREG to support 64-bit extensions..

If the extend type is 64-bits, emit a 32-bit -> 64-bit extend after the UDIVREM8_ZEXT_HREG/UDIVREM8_SEXT_HREG operation.

This gives a shorter encoding for the second extend in the sext case, and allows us to completely remove the second extend in the zext case.

craig.topper retitled this revision from [X86] Imiprove handling of UDIVREM8_ZEXT_HREG/UDIVREM8_SEXT_HREG to support 64-bit extensions. to [X86] Improve handling of UDIVREM8_ZEXT_HREG/UDIVREM8_SEXT_HREG to support 64-bit extensions..Oct 3 2017, 2:24 PM
craig.topper retitled this revision from [X86] Improve handling of UDIVREM8_ZEXT_HREG/UDIVREM8_SEXT_HREG to support 64-bit extensions. to [X86] Improve handling of UDIVREM8_ZEXT_HREG/SDIVREM8_SEXT_HREG to support 64-bit extensions..

Add known and sign bits support for UDIVREM8_ZEXT_HREG/SDIVREM8_SEXT_HREG. The UDIVREM8_ZEXT_HREG is needed to prevent a regression on one of the test cases where an extra setb was being generated.

RKSimon accepted this revision.Oct 26 2017, 2:44 AM

LGTM - possibly split the sign/known bits change from the X86ISelDAGToDAG.cpp change but its not vital to do so

This revision is now accepted and ready to land.Oct 26 2017, 2:44 AM
This revision was automatically updated to reflect the committed changes.