[PATCH] D71178: [x86] Don't assume sign-extension of arguments smaller than 32-bits.

[Emilio Cobos Álvarez via Phabricator via llvm-commits]

emilio created this revision.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.
emilio added a comment.
xbolva00 added reviewers: craig.topper, RKSimon.

Could I get some feedback on this? Does this look reasonable?


Even though llvm currently will sign-extend in the caller, this is not
guaranteed by the ABI, and thus the callee cannot assume it.

This fixes PR44228 and PR12207.

I still have a few tests to update, but I want to sanity-check this is
reasonable before doing so.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71178

Files:
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/test/CodeGen/X86/addr-mode-matcher-2.ll
  llvm/test/CodeGen/X86/avx512-regcall-NoMask.ll
  llvm/test/CodeGen/X86/mask-negated-bool.ll
  llvm/test/CodeGen/X86/x86-64-arg.ll


Index: llvm/test/CodeGen/X86/x86-64-arg.ll
===================================================================
--- llvm/test/CodeGen/X86/x86-64-arg.ll
+++ /dev/null
@@ -1,15 +0,0 @@
-; RUN: llc < %s | grep "movl	%edi, %eax"
-; The input value is already sign extended, don't re-extend it.
-; This testcase corresponds to:
-;   int test(short X) { return (int)X; }
-
-target datalayout = "e-p:64:64"
-target triple = "x86_64-apple-darwin8"
-
-
-define i32 @test(i16 signext  %X) {
-entry:
-        %tmp12 = sext i16 %X to i32             ; <i32> [#uses=1]
-        ret i32 %tmp12
-}
-
Index: llvm/test/CodeGen/X86/mask-negated-bool.ll
===================================================================
--- llvm/test/CodeGen/X86/mask-negated-bool.ll
+++ llvm/test/CodeGen/X86/mask-negated-bool.ll
@@ -16,7 +16,7 @@
 define i32 @mask_negated_zext_bool2(i1 zeroext %x) {
 ; CHECK-LABEL: mask_negated_zext_bool2:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    movl %edi, %eax
+; CHECK-NEXT:    movzbl %dil, %eax
 ; CHECK-NEXT:    retq
   %ext = zext i1 %x to i32
   %neg = sub i32 0, %ext
@@ -50,7 +50,7 @@
 define i32 @mask_negated_sext_bool2(i1 zeroext %x) {
 ; CHECK-LABEL: mask_negated_sext_bool2:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    movl %edi, %eax
+; CHECK-NEXT:    movzbl %dil, %eax
 ; CHECK-NEXT:    retq
   %ext = sext i1 %x to i32
   %neg = sub i32 0, %ext
Index: llvm/test/CodeGen/X86/avx512-regcall-NoMask.ll
===================================================================
--- llvm/test/CodeGen/X86/avx512-regcall-NoMask.ll
+++ llvm/test/CodeGen/X86/avx512-regcall-NoMask.ll
@@ -1253,13 +1253,15 @@
 ; LINUXOSX64:       # %bb.0:
 ; LINUXOSX64-NEXT:    vcvtss2sd %xmm1, %xmm1, %xmm1
 ; LINUXOSX64-NEXT:    vaddsd %xmm0, %xmm1, %xmm0
+; LINUXOSX64-NEXT:    movsbl %al, %eax
 ; LINUXOSX64-NEXT:    vcvtsi2sd %eax, %xmm2, %xmm1
 ; LINUXOSX64-NEXT:    vaddsd %xmm1, %xmm0, %xmm0
 ; LINUXOSX64-NEXT:    vcvtsi2sd %ecx, %xmm2, %xmm1
 ; LINUXOSX64-NEXT:    vaddsd %xmm1, %xmm0, %xmm0
 ; LINUXOSX64-NEXT:    vcvtsi2sd %rdx, %xmm2, %xmm1
 ; LINUXOSX64-NEXT:    vaddsd %xmm1, %xmm0, %xmm0
-; LINUXOSX64-NEXT:    vcvtsi2sd %edi, %xmm2, %xmm1
+; LINUXOSX64-NEXT:    movswl %di, %eax
+; LINUXOSX64-NEXT:    vcvtsi2sd %eax, %xmm2, %xmm1
 ; LINUXOSX64-NEXT:    vaddsd %xmm1, %xmm0, %xmm0
 ; LINUXOSX64-NEXT:    vcvtsi2sdl (%rsi), %xmm2, %xmm1
 ; LINUXOSX64-NEXT:    vaddsd %xmm1, %xmm0, %xmm0
Index: llvm/test/CodeGen/X86/addr-mode-matcher-2.ll
===================================================================
--- llvm/test/CodeGen/X86/addr-mode-matcher-2.ll
+++ llvm/test/CodeGen/X86/addr-mode-matcher-2.ll
@@ -42,7 +42,7 @@
 ; X64-LABEL: foo:
 ; X64:       # %bb.0:
 ; X64-NEXT:    pushq %rax
-; X64-NEXT:    testl %edi, %edi
+; X64-NEXT:    testb %dil, %dil
 ; X64-NEXT:    je .LBB0_1
 ; X64-NEXT:  # %bb.3:
 ; X64-NEXT:    popq %rax
Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===================================================================
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -3352,16 +3352,7 @@
         ArgValue = DAG.getCopyFromReg(Chain, dl, Reg, RegVT);
       }
 
-      // If this is an 8 or 16-bit value, it is really passed promoted to 32
-      // bits.  Insert an assert[sz]ext to capture this, then truncate to the
-      // right size.
-      if (VA.getLocInfo() == CCValAssign::SExt)
-        ArgValue = DAG.getNode(ISD::AssertSext, dl, RegVT, ArgValue,
-                               DAG.getValueType(VA.getValVT()));
-      else if (VA.getLocInfo() == CCValAssign::ZExt)
-        ArgValue = DAG.getNode(ISD::AssertZext, dl, RegVT, ArgValue,
-                               DAG.getValueType(VA.getValVT()));
-      else if (VA.getLocInfo() == CCValAssign::BCvt)
+      if (VA.getLocInfo() == CCValAssign::BCvt)
         ArgValue = DAG.getBitcast(VA.getValVT(), ArgValue);
 
       if (VA.isExtInLoc()) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D71178.232748.patch
Type: text/x-patch
Size: 3889 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191209/d448e6ec/attachment.bin>