[GlobalISel][CallLowering] Use hasRetAttr for return flags on CallBases

[paquette]

Summary

Given something like this:

declare signext i16 @signext_callee()
define i32 @caller() {
%res = call i16 @signext_callee()
}

CallLowering would miss that signext_callee's return value is sign extended, because it isn't on the call.

Use hasRetAttr on the CallBase to allow us to catch this.

[arsenm]

The change itself is fine and more consistent, but this is just making an existing problem worse. Instead of treating signext/zeroext as optimization hints on arguments and call results, we're actually introducing code to do the extension which we were promised was already done. We really should be emitting the equivalent of ISD::AssertZext/AssertSext

llvm/test/CodeGen/AArch64/GlobalISel/call-translator.ll

392

This doesn't actually need a G_ZEXT. In this context it's supposed to be an optimization hint, so really this should be something like

; CHECK:   [[COPY:%[0-9]+]]:_(s32) = COPY $w0
; CHECK: [[ZEXT_HINT]]:_(s32) = G_ASSERT_ZEXT [[COPY]]

[paquette]

In D86228#2226562, @arsenm wrote:

The change itself is fine and more consistent, but this is just making an existing problem worse. Instead of treating signext/zeroext as optimization hints on arguments and call results, we're actually introducing code to do the extension which we were promised was already done. We really should be emitting the equivalent of ISD::AssertZext/AssertSext

Yeah, I'm actually working on that right now. The problem is that right now, in, say, this case:

declare signext i16 @signext_callee()
define i32 @foo() {
  %res = call i16 @signext_callee()
  %ext = sext i16 %res to i32
  ret i32 %ext
}

We wouldn't even know that we could emit a G_ASSERT_SEXT. All this patch is supposed to do is make sure that we get the attribute plumbing correct.

[aemerson]

Agreed that adding G_ASSERT_{ZEXT,SEXT} would be useful. We can teach known bits about them and eliminate superfluous extends in the prelegalizer combiner.

llvm/include/llvm/CodeGen/GlobalISel/CallLowering.h

template <class XXXTargetLowering>
     return static_cast<const XXXTargetLowering *>(TLI);
   }
 
   /// \returns Flags corresponding to the attributes on the \p ArgIdx-th
   /// parameter of \p Call.
   ISD::ArgFlagsTy getAttributesForArgIdx(const CallBase &Call,
                                          unsigned ArgIdx) const;
 
+  /// \returns Flags corresponding to the attributes on the return from \p Call.
+  ISD::ArgFlagsTy getAttributesForReturn(const CallBase &Call) const;
+
   /// Adds flags to \p Flags based off of the attributes in \p Attrs.
   /// \p OpIdx is the index in \p Attrs to add flags from.
   void addArgFlagsFromAttributes(ISD::ArgFlagsTy &Flags,
                                  const AttributeList &Attrs,
                                  unsigned OpIdx) const;
 
   template <typename FuncInfoTy>
   void setArgFlags(ArgInfo &Arg, unsigned OpIdx, const DataLayout &DL,

llvm/lib/CodeGen/GlobalISel/CallLowering.cpp

ISD::ArgFlagsTy CallLowering::getAttributesForArgIdx(const CallBase &Call,
                                                      unsigned ArgIdx) const {
   ISD::ArgFlagsTy Flags;
   addFlagsUsingAttrFn(Flags, [&Call, &ArgIdx](Attribute::AttrKind Attr) {
     return Call.paramHasAttr(ArgIdx, Attr);
   });
   return Flags;
 }
 
+ISD::ArgFlagsTy
+CallLowering::getAttributesForReturn(const CallBase &Call) const {
+  ISD::ArgFlagsTy Flags;
+  addFlagsUsingAttrFn(Flags, [&Call](Attribute::AttrKind Attr) {
+    return Call.hasRetAttr(Attr);
+  });
+  return Flags;
+}
+
 void CallLowering::addArgFlagsFromAttributes(ISD::ArgFlagsTy &Flags,
                                              const AttributeList &Attrs,
                                              unsigned OpIdx) const {
   addFlagsUsingAttrFn(Flags, [&Attrs, &OpIdx](Attribute::AttrKind Attr) {
     return Attrs.hasAttribute(OpIdx, Attr);
   });
 }
 
bool CallLowering::lowerCall(MachineIRBuilder &MIRBuilder, const CallBase &CB,
   // Try looking through a bitcast from one function type to another.
   // Commonly happens with calls to objc_msgSend().
   const Value *CalleeV = CB.getCalledOperand()->stripPointerCasts();
   if (const Function *F = dyn_cast<Function>(CalleeV))
     Info.Callee = MachineOperand::CreateGA(F, 0);
   else
     Info.Callee = MachineOperand::CreateReg(GetCalleeReg(), false);
 
-  Info.OrigRet = ArgInfo{ResRegs, CB.getType(), ISD::ArgFlagsTy{}};
+  Info.OrigRet = ArgInfo{ResRegs, CB.getType(), getAttributesForReturn(CB)};
   if (!Info.OrigRet.Ty->isVoidTy())
     setArgFlags(Info.OrigRet, AttributeList::ReturnIndex, DL, CB);
 
   Info.KnownCallees = CB.getMetadata(LLVMContext::MD_callees);
   Info.CallConv = CB.getCallingConv();
   Info.SwiftErrorVReg = SwiftErrorVReg;
   Info.IsMustTailCall = CB.isMustTailCall();
   Info.IsTailCall = CanBeTailCalled;

llvm/test/CodeGen/AArch64/GlobalISel/call-translator.ll

 ; CHECK-NEXT: BL
 ; CHECK-NEXT: ADJCALLSTACKUP
 ; CHECK-NEXT: RET_ReallyLR
 define void @call_returns_array_size0_struct() {
   ; FIXME: Why is this valid IR?
   %call = call [1 x %size0type] @func.returns.array.size0.struct()
   ret void
 }
+
+; Test extends on return values.
+
+; This should notice that the return value from has_zext_return is zero
+; extended.
+declare zeroext i16 @has_zext_return()
+define i32 @test_zext_return_from_callee() {
+  ; CHECK-LABEL: name: test_zext_return_from_callee
+  ; CHECK: bb.1 (%ir-block.0):
+  ; CHECK:   ADJCALLSTACKDOWN 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK:   BL @has_zext_return, csr_aarch64_aapcs, implicit-def $lr, implicit $sp, implicit-def $w0
+  ; CHECK:   [[COPY:%[0-9]+]]:_(s32) = COPY $w0
+  ; CHECK:   [[TRUNC:%[0-9]+]]:_(s16) = G_TRUNC [[COPY]](s32)
+  ; CHECK:   ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK:   [[ZEXT:%[0-9]+]]:_(s32) = G_ZEXT [[TRUNC]](s16)
+  ; CHECK:   $w0 = COPY [[ZEXT]](s32)
+  ; CHECK:   RET_ReallyLR implicit $w0
+  %val = call i16 @has_zext_return()
+  %ext = zext i16 %val to i32
+  ret i32 %ext
+}
+
+; Same as above, but with zeroext explicitly on the call. Should produce the
+; same codegen.
+define i32 @test_zext_return_from_callee2() {
+  ; CHECK-LABEL: name: test_zext_return_from_callee2
+  ; CHECK: bb.1 (%ir-block.0):
+  ; CHECK:   ADJCALLSTACKDOWN 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK:   BL @has_zext_return, csr_aarch64_aapcs, implicit-def $lr, implicit $sp, implicit-def $w0
+  ; CHECK:   [[COPY:%[0-9]+]]:_(s32) = COPY $w0
+  ; CHECK:   [[TRUNC:%[0-9]+]]:_(s16) = G_TRUNC [[COPY]](s32)
+  ; CHECK:   ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK:   [[ZEXT:%[0-9]+]]:_(s32) = G_ZEXT [[TRUNC]](s16)
+  ; CHECK:   $w0 = COPY [[ZEXT]](s32)
+  ; CHECK:   RET_ReallyLR implicit $w0
+  %val = call zeroext i16 @has_zext_return()
+  %ext = zext i16 %val to i32
+  ret i32 %ext
+}
+
+; This should notice that the return value from has_sext_return is sign
+; extended.
+declare signext i16 @has_sext_return()
+define i32 @test_sext_return_from_callee() {
+  ; CHECK-LABEL: name: test_sext_return_from_callee
+  ; CHECK: bb.1 (%ir-block.0):
+  ; CHECK:   ADJCALLSTACKDOWN 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK:   BL @has_sext_return, csr_aarch64_aapcs, implicit-def $lr, implicit $sp, implicit-def $w0
+  ; CHECK:   [[COPY:%[0-9]+]]:_(s32) = COPY $w0
+  ; CHECK:   [[TRUNC:%[0-9]+]]:_(s16) = G_TRUNC [[COPY]](s32)
+  ; CHECK:   ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK:   [[SEXT:%[0-9]+]]:_(s32) = G_SEXT [[TRUNC]](s16)
+  ; CHECK:   $w0 = COPY [[SEXT]](s32)
+  ; CHECK:   RET_ReallyLR implicit $w0
+  %val = call i16 @has_sext_return()
+  %ext = sext i16 %val to i32
+  ret i32 %ext
+}
+
+; Same as above, but with signext explicitly on the call. Should produce the
+; same codegen.
+define i32 @test_sext_return_from_callee2() {
+  ; CHECK-LABEL: name: test_sext_return_from_callee2
+  ; CHECK: bb.1 (%ir-block.0):
+  ; CHECK:   ADJCALLSTACKDOWN 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK:   BL @has_sext_return, csr_aarch64_aapcs, implicit-def $lr, implicit $sp, implicit-def $w0
+  ; CHECK:   [[COPY:%[0-9]+]]:_(s32) = COPY $w0
+  ; CHECK:   [[TRUNC:%[0-9]+]]:_(s16) = G_TRUNC [[COPY]](s32)
+  ; CHECK:   ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK:   [[SEXT:%[0-9]+]]:_(s32) = G_SEXT [[TRUNC]](s16)
+  ; CHECK:   $w0 = COPY [[SEXT]](s32)
+  ; CHECK:   RET_ReallyLR implicit $w0
+  %val = call signext i16 @has_sext_return()
+  %ext = sext i16 %val to i32
+  ret i32 %ext
+}