[InstCombine] Simplify calls with casted "returned" attribute

[jdoerfert]

Summary

The handling of the returned attribute in D75815 did miss the case
where the argument is (bit)casted to a different type. This is
explicitly allowed by the language reference and exposed by the
Attributor.

[lebedev.ri]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

4662

Why only pointers?
https://llvm.org/docs/LangRef.html#parameter-attributes

returned

This indicates that the function always returns the argument as its return value. <...> The parameter and the function return type must be valid operands for the bitcast instruction.

[jdoerfert]

Use canLosslesslyBitCastTo as suggested by @lebedev.ri and test it via vector casts

[lebedev.ri]

LG

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

4660

I can't imagine it matters, but these should be swapped around.

[nikic]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

4660

Shouldn't this be an assertion rather than an if check?

[lebedev.ri]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

4660

Is there a verifier check for that?

[jdoerfert]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

4660

@nikic has a point. To fix the issue we just need the CreateBitOrPointerCast, anything that is not valid is an error in the IR.
@lebedev.ri are you sure wrt. the order.We cast the returned argument to the call type so I figured we want to know if the returned argument type can be casted to the call return type.

I think I'll update with an assertion.

[xbolva00]

llvm/test/Transforms/InstCombine/call-returned.ll

58

I am wondering if these all cases can be “reversed” to exploit tail call optimization.

[lebedev.ri]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

4660

@lebedev.ri are you sure wrt. the order.We cast the returned argument to the call type so I figured we want to know if the returned argument type can be casted to the call return type.

Hm, i guess it depends on how you look at it, either one can be argued to be the right one.

[lebedev.ri]

I'd prefer for the current patch to proceed as-is first, and for things to be refactored later.

<jdoerfert> LebedevRI: are u OK with the assertion instead of the conditional?
<LebedevRI> if we want to make that an assertion, we should do it in verifier
<LebedevRI> and avoid assertion there at all
<LebedevRI> (there == in the patch at hand)
<LebedevRI> but both the assertion and verifier may find things, so you might want to proceed with the current patch first and refactor later
<jdoerfert> can u add that comment there please. I'll do that then

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

Instruction *InstCombiner::visitCallBase(CallBase &Call) {
   if (CallInst *CI = dyn_cast<CallInst>(&Call)) {
     Instruction *I = tryOptimizeCall(CI);
     // If we changed something return the result, etc. Otherwise let
     // the fallthrough check.
     if (I) return eraseInstFromFunction(*I);
   }
 
   if (!Call.use_empty() && !Call.isMustTailCall())
-    if (Value *ReturnedArg = Call.getReturnedArgOperand())
-      return replaceInstUsesWith(Call, ReturnedArg);
+    if (Value *ReturnedArg = Call.getReturnedArgOperand()) {
+      Type *CallTy = Call.getType();
+      Type *RetArgTy = ReturnedArg->getType();
+      if (RetArgTy->canLosslesslyBitCastTo(CallTy))
+        return replaceInstUsesWith(
+            Call, Builder.CreateBitOrPointerCast(ReturnedArg, CallTy));
+    }
 
   if (isAllocLikeFn(&Call, &TLI))
     return visitAllocSite(Call);
 
   return Changed ? &Call : nullptr;
 }
 
 /// If the callee is a constexpr cast of a function, attempt to move the cast to

llvm/test/Transforms/InstCombine/call-returned.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -S -instcombine < %s | FileCheck %s
 
 declare i32 @passthru_i32(i32 returned)
 declare i8* @passthru_p8(i8* returned)
+declare i8* @passthru_p8_from_p32(i32* returned)
+declare <8 x i8> @passthru_8i8v_from_2i32v(<2 x i32> returned)
 
 define i32 @returned_const_int_arg() {
 ; CHECK-LABEL: @returned_const_int_arg(
 ; CHECK-NEXT:    [[X:%.*]] = call i32 @passthru_i32(i32 42)
 ; CHECK-NEXT:    ret i32 42
 ;
   %x = call i32 @passthru_i32(i32 42)
   ret i32 %x
 }
 
 define i8* @returned_const_ptr_arg() {
 ; CHECK-LABEL: @returned_const_ptr_arg(
 ; CHECK-NEXT:    [[X:%.*]] = call i8* @passthru_p8(i8* null)
 ; CHECK-NEXT:    ret i8* null
 ;
   %x = call i8* @passthru_p8(i8* null)
   ret i8* %x
 }
 
+define i8* @returned_const_ptr_arg_casted() {
+; CHECK-LABEL: @returned_const_ptr_arg_casted(
+; CHECK-NEXT:    [[X:%.*]] = call i8* @passthru_p8_from_p32(i32* null)
+; CHECK-NEXT:    ret i8* null
+;
+  %x = call i8* @passthru_p8_from_p32(i32* null)
+  ret i8* %x
+}
+
+define i8* @returned_ptr_arg_casted(i32* %a) {
+; CHECK-LABEL: @returned_ptr_arg_casted(
+; CHECK-NEXT:    [[TMP1:%.*]] = bitcast i32* [[A:%.*]] to i8*
+; CHECK-NEXT:    [[X:%.*]] = call i8* @passthru_p8_from_p32(i32* [[A]])
+; CHECK-NEXT:    ret i8* [[TMP1]]
+;
+  %x = call i8* @passthru_p8_from_p32(i32* %a)
+  ret i8* %x
+}
+
+@GV = constant <2 x i32> zeroinitializer
+define <8 x i8> @returned_const_vec_arg_casted() {
+; CHECK-LABEL: @returned_const_vec_arg_casted(
+; CHECK-NEXT:    [[X:%.*]] = call <8 x i8> @passthru_8i8v_from_2i32v(<2 x i32> zeroinitializer)
+; CHECK-NEXT:    ret <8 x i8> zeroinitializer
+;
+  %v = load <2 x i32>, <2 x i32>* @GV
+  %x = call <8 x i8> @passthru_8i8v_from_2i32v(<2 x i32> %v)
+  ret <8 x i8> %x
+}
+
+define <8 x i8> @returned_vec_arg_casted(<2 x i32> %a) {
+; CHECK-LABEL: @returned_vec_arg_casted(
+; CHECK-NEXT:    [[TMP1:%.*]] = bitcast <2 x i32> [[A:%.*]] to <8 x i8>
+; CHECK-NEXT:    [[X:%.*]] = call <8 x i8> @passthru_8i8v_from_2i32v(<2 x i32> [[A]])
+; CHECK-NEXT:    ret <8 x i8> [[TMP1]]
+;
+  %x = call <8 x i8> @passthru_8i8v_from_2i32v(<2 x i32> %a)
+  ret <8 x i8> %x
+}
+
 define i32 @returned_var_arg(i32 %arg) {
 ; CHECK-LABEL: @returned_var_arg(
 ; CHECK-NEXT:    [[X:%.*]] = call i32 @passthru_i32(i32 [[ARG:%.*]])
 ; CHECK-NEXT:    ret i32 [[ARG]]
 ;
   %x = call i32 @passthru_i32(i32 %arg)
   ret i32 %x
 }

 define i32 @returned_var_arg_musttail(i32 %arg) {
 ; CHECK-LABEL: @returned_var_arg_musttail(
 ; CHECK-NEXT:    [[X:%.*]] = musttail call i32 @passthru_i32(i32 [[ARG:%.*]])
 ; CHECK-NEXT:    ret i32 [[X]]
 ;
   %x = musttail call i32 @passthru_i32(i32 %arg)
   ret i32 %x
 }
+