[PATCH] D45186: [InstCombine] Don't optimize call target casts inside thunks

[Reid Kleckner via Phabricator via llvm-commits]

rnk created this revision.
rnk added reviewers: efriedma, majnemer, pcc.
Herald added a subscriber: hiraditya.

Thunks are functions that take a varargs pack and forward all their
unprototyped arguments to some other function. This is done with a
variadic musttail call, which forwards all the remaining unprototyped
argument registers to the tail call.

Instcombine shouldn't try to modify call targets casts inside musttail
thunks, or it will break the verifier requirement that musttail call
targets match the prototype of the caller.


https://reviews.llvm.org/D45186

Files:
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/test/Transforms/InstCombine/musttail-thunk.ll


Index: llvm/test/Transforms/InstCombine/musttail-thunk.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/InstCombine/musttail-thunk.ll
@@ -0,0 +1,32 @@
+; RUN: opt -instcombine -S < %s | FileCheck %s
+
+; These are both direct calls, but make sure instcombine leaves the casts
+; alone.
+
+define i32 @call_thunk(i32 %x, i32 %y) {
+  %r = call i32 bitcast (void (i32, ...)* @inc_first_arg_thunk to i32 (i32, i32)*)(i32 %x, i32 %y)
+  ret i32 %r
+}
+
+; CHECK-LABEL: define i32 @call_thunk(i32 %x, i32 %y)
+; CHECK:   %r = call i32 bitcast (void (i32, ...)* @inc_first_arg_thunk to i32 (i32, i32)*)(i32 %x, i32 %y)
+; CHECK:   ret i32 %r
+
+define internal void @inc_first_arg_thunk(i32 %arg1, ...) #0 {
+entry:
+  %inc = add i32 %arg1, 1
+  musttail call void (i32, ...) bitcast (i32 (i32, i32)* @plus to void (i32, ...)*)(i32 %inc, ...)
+  ret void
+}
+
+; CHECK-LABEL: define internal void @inc_first_arg_thunk(i32 %arg1, ...) #0
+; CHECK:   %inc = add i32 %arg1, 1
+; CHECK:   musttail call void (i32, ...) bitcast (i32 (i32, i32)* @plus to void (i32, ...)*)(i32 %inc, ...)
+; CHECK:   ret void
+
+define internal i32 @plus(i32 %x, i32 %y) {
+  %r = add i32 %x, %y
+  ret i32 %r
+}
+
+attributes #0 = { "thunk" }
Index: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -3996,11 +3996,21 @@
   if (!Callee)
     return false;
 
-  // The prototype of a thunk is a lie. Don't directly call such a function.
+  // If this is a call to a thunk function, don't remove the cast. Thunks are
+  // used to transparently forward all incoming parameters and outgoing return
+  // values, so it's important to leave the cast in place.
   if (Callee->hasFnAttribute("thunk"))
     return false;
 
   Instruction *Caller = CS.getInstruction();
+
+  // For the same reasons, if this is a musttail call in a thunk, don't remove
+  // the cast.
+  CallInst *CI = dyn_cast<CallInst>(Caller);
+  if (CI && CI->isMustTailCall() &&
+      CI->getParent()->getParent()->hasFnAttribute("thunk"))
+    return false;
+
   const AttributeList &CallerPAL = CS.getAttributes();
 
   // Okay, this is a cast from a function to a different type.  Unless doing so


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D45186.140687.patch
Type: text/x-patch
Size: 2386 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180402/c654e2f3/attachment-0001.bin>