[PATCH] Allow reasonable zeroext/signext tail calls

[Tim Northover]

Hi nicholas,

As in PR16742, we should probably be able to handle tail calls like:
    bool g(void); 
    bool f() { return g(); }

At the moment, since @f ends up returning "zeroext i1" we immediately bail out of trying to make it a tail call. In fact provided @f and @g return the *same* sized result via an extension, the extension required will itself will be the same.

This patch implements that logic:
+ If the caller (@f) has no extension attribute, anything goes, as before -- the higher bits are undef.
+ If the caller has zeroext/signext and the callee has different, then things will go wrong.
+ Otherwise, the conventions match so make sure the bit-width is the same in each case.

http://llvm-reviews.chandlerc.com/D1299

Files:
  lib/CodeGen/Analysis.cpp
  test/CodeGen/X86/sibcall.ll
  test/CodeGen/X86/tail-call-exts.ll

Index: lib/CodeGen/Analysis.cpp
===================================================================
--- lib/CodeGen/Analysis.cpp
+++ lib/CodeGen/Analysis.cpp
@@ -320,6 +320,7 @@
 static bool slotOnlyDiscardsData(const Value *RetVal, const Value *CallVal,
                                  SmallVectorImpl<unsigned> &RetIndices,
                                  SmallVectorImpl<unsigned> &CallIndices,
+                                 bool AllowDifferingSizes,
                                  const TargetLoweringBase &TLI) {
 
   // Trace the sub-value needed by the return value as far back up the graph as
@@ -350,7 +351,8 @@
   // all the bits that are needed by the "ret" have been provided by the "tail
   // call". FIXME: with sufficiently cunning bit-tracking, we could look through
   // extensions too.
-  if (BitsProvided < BitsRequired)
+  if (BitsProvided < BitsRequired ||
+      (!AllowDifferingSizes && BitsProvided != BitsRequired))
     return false;
 
   return true;
@@ -517,20 +519,25 @@
   // return type is.
   if (isa<UndefValue>(Ret->getOperand(0))) return true;
 
-  // Conservatively require the attributes of the call to match those of
-  // the return. Ignore noalias because it doesn't affect the call sequence.
-  const Function *F = ExitBB->getParent();
-  AttributeSet CallerAttrs = F->getAttributes();
-  if (AttrBuilder(CallerAttrs, AttributeSet::ReturnIndex).
-        removeAttribute(Attribute::NoAlias) !=
-      AttrBuilder(CallerAttrs, AttributeSet::ReturnIndex).
-        removeAttribute(Attribute::NoAlias))
-    return false;
+  // If the ABI expects the caller to do some extension of the result before
+  // return, then in general we shouldn't allow a truncate between the call-site
+  // and the ret.
+  AttributeSet CallerAttrs = ExitBB->getParent()->getAttributes();
+  AttributeSet CalleeAttrs = cast<CallInst>(I)->getAttributes();
 
-  // It's not safe to eliminate the sign / zero extension of the return value.
-  if (CallerAttrs.hasAttribute(AttributeSet::ReturnIndex, Attribute::ZExt) ||
-      CallerAttrs.hasAttribute(AttributeSet::ReturnIndex, Attribute::SExt))
-    return false;
+  bool AllowDifferingSizes = true;
+  if (CallerAttrs.hasAttribute(AttributeSet::ReturnIndex, Attribute::ZExt)) {
+    if (!CalleeAttrs.hasAttribute(AttributeSet::ReturnIndex, Attribute::ZExt))
+      return false;
+
+    AllowDifferingSizes = false;
+  } else if (CallerAttrs.hasAttribute(AttributeSet::ReturnIndex,
+                                      Attribute::SExt)) {
+    if (!CalleeAttrs.hasAttribute(AttributeSet::ReturnIndex, Attribute::SExt))
+      return false;
+
+    AllowDifferingSizes = false;
+  }
 
   const Value *RetVal = Ret->getOperand(0), *CallVal = I;
   SmallVector<unsigned, 4> RetPath, CallPath;
@@ -571,7 +578,8 @@
 
     // Finally, we can check whether the value produced by the tail call at this
     // index is compatible with the value we return.
-    if (!slotOnlyDiscardsData(RetVal, CallVal, TmpRetPath, TmpCallPath, TLI))
+    if (!slotOnlyDiscardsData(RetVal, CallVal, TmpRetPath, TmpCallPath,
+                              AllowDifferingSizes, TLI))
       return false;
 
     CallEmpty  = !nextRealType(CallSubTypes, CallPath);
Index: test/CodeGen/X86/sibcall.ll
===================================================================
--- test/CodeGen/X86/sibcall.ll
+++ test/CodeGen/X86/sibcall.ll
@@ -106,10 +106,10 @@
 define signext i16 @t8() nounwind ssp {
 entry:
 ; 32-LABEL: t8:
-; 32: calll {{_?}}bar3
+; 32: jmp {{_?}}bar3
 
 ; 64-LABEL: t8:
-; 64: callq {{_?}}bar3
+; 64: jmp {{_?}}bar3
   %0 = tail call signext i16 @bar3() nounwind      ; <i16> [#uses=1]
   ret i16 %0
 }
@@ -122,7 +122,7 @@
 ; 32: calll *
 
 ; 64-LABEL: t9:
-; 64: callq *
+; 64: jmpq *
   %0 = bitcast i32 (i32)* %x to i16 (i32)*
   %1 = tail call signext i16 %0(i32 0) nounwind
   ret i16 %1
Index: test/CodeGen/X86/tail-call-exts.ll
===================================================================
--- /dev/null
+++ test/CodeGen/X86/tail-call-exts.ll
@@ -0,0 +1,39 @@
+; RUN: llc -mtriple=x86_64-apple-darwin -o - %s | FileCheck %s
+
+; Simple case: completely identical returns, even with extensions, shouldn't be
+; a barrier to tail calls.
+declare zeroext i1 @give_bool()
+define zeroext i1 @test_bool() {
+; CHECK-LABEL: test_bool:
+; CHECK: jmp
+  %call = tail call zeroext i1 @give_bool()
+  ret i1 %call
+}
+
+; Here, there's more zero extension to be done between the call and the return,
+; so a tail call is impossible (well, according to current Clang practice
+; anyway. The AMD64 ABI isn't crystal clear on the matter).
+declare zeroext i32 @give_i32()
+define zeroext i8 @test_i32() {
+; CHECK-LABEL: test_i32:
+; CHECK: callq _give_i32
+; CHECK: movzbl %al, %eax
+; CHECK: ret
+
+  %call = tail call zeroext i32 @give_i32()
+  %val = trunc i32 %call to i8
+  ret i8 %val
+}
+
+; Here, one function is zeroext and the other is signext. To the extent that
+; these both mean something they are incompatible so no tail call is possible.
+declare zeroext i16 @give_unsigned_i16()
+define signext i16 @test_incompatible_i16() {
+; CHECK-LABEL: test_incompatible_i16:
+; CHECK: callq _give_unsigned_i16
+; CHECK: cwtl
+; CHECK: ret
+
+  %call = tail call zeroext i16 @give_unsigned_i16()
+  ret i16 %call
+}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1299.1.patch
Type: text/x-patch
Size: 5290 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130806/1b2c8332/attachment.bin>