[OpaquePtr] Use ArgListEntry indirect types more in ISel lowering

[aeubanks]

Summary

To avoid calling PointerType::getElementType().

Without also examining the callee in Call->getParam*Type(), we may end
up seeing that we have a byval attribute on the callee, but only looking
at the caller argument attributes which could be missing the byval
attribute.

[dblaikie]

llvm/include/llvm/IR/InstrTypes.h

1749–1750

When does the 'Ty' come back as null? Would that only be it older bitcode, that should be auto-upgraded when loading?

[aeubanks]

assume indirectly passed types are always available

[aeubanks]

llvm/include/llvm/IR/InstrTypes.h

1749–1750

ah you're right
there was one test that was failing when assuming the type was never null, turns out we need to check more closely for these attributes on the arguments, some tests were missing that: https://reviews.llvm.org/D101725

[dblaikie]

Looks good, contingent on D101725, thanks!

[aeubanks]

rebase

[aeubanks]

rebase
check callee for byval/preallocated/inalloca type

[aeubanks]

rebase

[rupprecht]

I don't see any messages in the reverts, but this change caused crashes both times it landed, in case that wasn't already the reason that it was reverted each time.

[aeubanks]

This was landed at some point

llvm/include/llvm/CodeGen/TargetLowering.h

public:
     bool IsInAlloca : 1;
     bool IsPreallocated : 1;
     bool IsReturned : 1;
     bool IsSwiftSelf : 1;
     bool IsSwiftAsync : 1;
     bool IsSwiftError : 1;
     bool IsCFGuardTarget : 1;
     MaybeAlign Alignment = None;
-    Type *ByValType = nullptr;
-    Type *PreallocatedType = nullptr;
+    Type *IndirectType = nullptr;
 
     ArgListEntry()
         : IsSExt(false), IsZExt(false), IsInReg(false), IsSRet(false),
           IsNest(false), IsByVal(false), IsByRef(false), IsInAlloca(false),
           IsPreallocated(false), IsReturned(false), IsSwiftSelf(false),
           IsSwiftAsync(false), IsSwiftError(false), IsCFGuardTarget(false) {}
 
     void setAttributes(const CallBase *Call, unsigned ArgIdx);

llvm/include/llvm/IR/InstrTypes.h

public:
   }
 
   MaybeAlign getParamStackAlign(unsigned ArgNo) const {
     return Attrs.getParamStackAlignment(ArgNo);
   }
 
   /// Extract the byval type for a call or parameter.
   Type *getParamByValType(unsigned ArgNo) const {
-    Type *Ty = Attrs.getParamByValType(ArgNo);
+    if (auto *Ty = Attrs.getParamByValType(ArgNo))
       return Ty;
+    if (const Function *F = getCalledFunction())
+      return F->getAttributes().getParamByValType(ArgNo);
+    return nullptr;
+  }
+
+  /// Extract the inalloca type for a call or parameter.
+  Type *getParamInAllocaType(unsigned ArgNo) const {
+    if (auto *Ty = Attrs.getParamInAllocaType(ArgNo))
+      return Ty;
+    if (const Function *F = getCalledFunction())
+      return F->getAttributes().getParamInAllocaType(ArgNo);
+    return nullptr;
   }
 
   /// Extract the preallocated type for a call or parameter.
   Type *getParamPreallocatedType(unsigned ArgNo) const {
-    Type *Ty = Attrs.getParamPreallocatedType(ArgNo);
+    if (auto *Ty = Attrs.getParamPreallocatedType(ArgNo))
       return Ty;
+    if (const Function *F = getCalledFunction())
+      return F->getAttributes().getParamPreallocatedType(ArgNo);
+    return nullptr;
   }
 
   /// Extract the number of dereferenceable bytes for a call or
   /// parameter (0=unknown).
   uint64_t getDereferenceableBytes(unsigned i) const {
     return Attrs.getDereferenceableBytes(i);
   }
 

llvm/lib/CodeGen/SelectionDAG/FastISel.cpp

for (unsigned I = 0, E = RetTys.size(); I != E; ++I) {
     }
   }
 
   // Handle all of the outgoing arguments.
   CLI.clearOuts();
   for (auto &Arg : CLI.getArgs()) {
     Type *FinalType = Arg.Ty;
     if (Arg.IsByVal)
-      FinalType = cast<PointerType>(Arg.Ty)->getElementType();
+      FinalType = Arg.IndirectType;
     bool NeedsRegBlock = TLI.functionArgumentNeedsConsecutiveRegisters(
         FinalType, CLI.CallConv, CLI.IsVarArg);
 
     ISD::ArgFlagsTy Flags;
     if (Arg.IsZExt)
       Flags.setZExt();
     if (Arg.IsSExt)
       Flags.setSExt();
if (Arg.IsPreallocated) {
       // preallocated. This way we can know how many bytes we should've
       // allocated and how many bytes a callee cleanup function will pop.  If we
       // port preallocated to more targets, we'll have to add custom
       // preallocated handling in the various CC lowering callbacks.
       Flags.setByVal();
     }
     MaybeAlign MemAlign = Arg.Alignment;
     if (Arg.IsByVal || Arg.IsInAlloca || Arg.IsPreallocated) {
-      PointerType *Ty = cast<PointerType>(Arg.Ty);
-      Type *ElementTy = Ty->getElementType();
-      unsigned FrameSize =
-          DL.getTypeAllocSize(Arg.ByValType ? Arg.ByValType : ElementTy);
+      Type *ElementTy = Arg.IndirectType;
+      unsigned FrameSize = DL.getTypeAllocSize(ElementTy);
 
       // For ByVal, alignment should come from FE. BE will guess if this info
       // is not there, but there are cases it cannot get right.
       if (!MemAlign)
         MemAlign = Align(TLI.getByValTypeAlignment(ElementTy, DL));
       Flags.setByValSize(FrameSize);
     } else if (!MemAlign) {
       MemAlign = DL.getABITypeAlign(Arg.Ty);

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

TargetLowering::LowerCallTo(TargetLowering::CallLoweringInfo &CLI) const {
   CLI.Outs.clear();
   CLI.OutVals.clear();
   for (unsigned i = 0, e = Args.size(); i != e; ++i) {
     SmallVector<EVT, 4> ValueVTs;
     ComputeValueVTs(*this, DL, Args[i].Ty, ValueVTs);
     // FIXME: Split arguments if CLI.IsPostTypeLegalization
     Type *FinalType = Args[i].Ty;
     if (Args[i].IsByVal)
-      FinalType = cast<PointerType>(Args[i].Ty)->getElementType();
+      FinalType = Args[i].IndirectType;
     bool NeedsRegBlock = functionArgumentNeedsConsecutiveRegisters(
         FinalType, CLI.CallConv, CLI.IsVarArg);
     for (unsigned Value = 0, NumValues = ValueVTs.size(); Value != NumValues;
          ++Value) {
       EVT VT = ValueVTs[Value];
       Type *ArgTy = VT.getTypeForEVT(CLI.RetTy->getContext());
       SDValue Op = SDValue(Args[i].Node.getNode(),
                            Args[i].Node.getResNo() + Value);
for (unsigned Value = 0, NumValues = ValueVTs.size(); Value != NumValues;
         // inalloca.  This way we can know how many bytes we should've allocated
         // and how many bytes a callee cleanup function will pop.  If we port
         // inalloca to more targets, we'll have to add custom inalloca handling
         // in the various CC lowering callbacks.
         Flags.setByVal();
       }
       Align MemAlign;
       if (Args[i].IsByVal || Args[i].IsInAlloca || Args[i].IsPreallocated) {
-        PointerType *Ty = cast<PointerType>(Args[i].Ty);
-        Type *ElementTy = Ty->getElementType();
+        Type *ElementTy = Args[i].IndirectType;
 
-        unsigned FrameSize = DL.getTypeAllocSize(
-            Args[i].ByValType ? Args[i].ByValType : ElementTy);
+        unsigned FrameSize = DL.getTypeAllocSize(ElementTy);
         Flags.setByValSize(FrameSize);
 
         // info is not there but there are cases it cannot get right.
         if (auto MA = Args[i].Alignment)
           MemAlign = *MA;
         else
           MemAlign = Align(getByValTypeAlignment(ElementTy, DL));
       } else if (auto MA = Args[i].Alignment) {

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

void TargetLoweringBase::ArgListEntry::setAttributes(const CallBase *Call,
   IsByVal = Call->paramHasAttr(ArgIdx, Attribute::ByVal);
   IsPreallocated = Call->paramHasAttr(ArgIdx, Attribute::Preallocated);
   IsInAlloca = Call->paramHasAttr(ArgIdx, Attribute::InAlloca);
   IsReturned = Call->paramHasAttr(ArgIdx, Attribute::Returned);
   IsSwiftSelf = Call->paramHasAttr(ArgIdx, Attribute::SwiftSelf);
   IsSwiftAsync = Call->paramHasAttr(ArgIdx, Attribute::SwiftAsync);
   IsSwiftError = Call->paramHasAttr(ArgIdx, Attribute::SwiftError);
   Alignment = Call->getParamStackAlign(ArgIdx);
-  ByValType = nullptr;
+  assert(IsByVal + IsInAlloca + IsPreallocated <= 1 &&
+         "can't have multiple indirect attributes");
+  IndirectType = nullptr;
   if (IsByVal) {
-    ByValType = Call->getParamByValType(ArgIdx);
+    IndirectType = Call->getParamByValType(ArgIdx);
+    assert(IndirectType && "no byval type?");
     if (!Alignment)
       Alignment = Call->getParamAlign(ArgIdx);
   }
-  PreallocatedType = nullptr;
-  if (IsPreallocated)
-    PreallocatedType = Call->getParamPreallocatedType(ArgIdx);
+  if (IsInAlloca) {
+    IndirectType = Call->getParamInAllocaType(ArgIdx);
+    assert(IndirectType && "no inalloca type?");
+  }
+
+  if (IsPreallocated) {
+    IndirectType = Call->getParamPreallocatedType(ArgIdx);
+    assert(IndirectType && "no preallocated type?");
+  }
 }
 
 /// Generate a libcall taking the given operands as arguments and returning a
 /// result of type RetVT.
 std::pair<SDValue, SDValue>
 TargetLowering::makeLibCall(SelectionDAG &DAG, RTLIB::Libcall LC, EVT RetVT,
                             ArrayRef<SDValue> Ops,
                             MakeLibCallOptions CallOptions,

llvm/test/CodeGen/Generic/mismatched-byval.ll

+; RUN: llc < %s -o /dev/null
+
+; Make sure we don't crash if the argument is missing the byval attribute
+
+declare void @f(i32* byval(i32))
+
+define void @g() {
+  %a = alloca i32
+  store i32 2, i32* %a
+  call void @f(i32* %a)
+  ret void
+}