[cfe-dev] Continuing Adventures with Objective-C

[Devang Patel]

On May 13, 2008, at 9:22 AM, David Chisnall wrote:

> expr.diff contains a load of small tidies (i.e. everything I  
> couldn't thing of which diff to put it in)

It is easier for everyone if you get such patches in as soon as you  
run into them.

> Index: lib/CodeGen/CGExprScalar.cpp
> ===================================================================
> --- lib/CodeGen/CGExprScalar.cpp	(revision 51026)
> +++ lib/CodeGen/CGExprScalar.cpp	(working copy)
> @@ -125,6 +125,7 @@
>      return EmitLoadOfLValue(E);
>    }
>    Value *VisitObjCMessageExpr(ObjCMessageExpr *E);
> +  Value *VisitObjCProtocolExpr(ObjCProtocolExpr *E);
>    Value *VisitObjCIvarRefExpr(ObjCIvarRefExpr *E) { return  
> EmitLoadOfLValue(E);}
>    Value *VisitArraySubscriptExpr(ArraySubscriptExpr *E);
>    Value *VisitMemberExpr(Expr *E)           { return  
> EmitLoadOfLValue(E); }
> @@ -298,6 +299,9 @@
>    Value *VisitChooseExpr(ChooseExpr *CE);
>    Value *VisitOverloadExpr(OverloadExpr *OE);
>    Value *VisitVAArgExpr(VAArgExpr *VE);
> +  Value *VisitObjCSelectorExpr(const ObjCSelectorExpr *E) {
> +    return CGF.EmitObjCSelectorExpr(E);
> +  }
>    Value *VisitObjCStringLiteral(const ObjCStringLiteral *E) {
>      return CGF.EmitObjCStringLiteral(E);
>    }

ok


> @@ -374,6 +378,9 @@
>    }
>
>    if (isa<PointerType>(SrcType)) {
> +    if(CGF.isObjCPointerType(DstType)) {
> +      return Builder.CreateBitCast(Src, DstTy, "conv");
> +    }

ok

>      // Must be an ptr to int cast.
>      assert(isa<llvm::IntegerType>(DstTy) && "not ptr->int?");
>      return Builder.CreatePtrToInt(Src, DstTy, "conv");
> @@ -452,46 +459,7 @@
>    return llvm::UndefValue::get(CGF.ConvertType(E->getType()));
>  }
>
> -Value *ScalarExprEmitter::VisitObjCMessageExpr(ObjCMessageExpr *E) {
> -  // Only the lookup mechanism and first two arguments of the method
> -  // implementation vary between runtimes.  We can get the receiver  
> and
> -  // arguments in generic code.
> -
> -  // Find the receiver
> -  llvm::Value *Receiver = CGF.EmitScalarExpr(E->getReceiver());
>
> -  // Process the arguments
> -  unsigned ArgC = E->getNumArgs();
> -  llvm::SmallVector<llvm::Value*, 16> Args;
> -  for (unsigned i = 0; i != ArgC; ++i) {
> -    Expr *ArgExpr = E->getArg(i);
> -    QualType ArgTy = ArgExpr->getType();
> -    if (!CGF.hasAggregateLLVMType(ArgTy)) {
> -      // Scalar argument is passed by-value.
> -      Args.push_back(CGF.EmitScalarExpr(ArgExpr));
> -    } else if (ArgTy->isAnyComplexType()) {
> -      // Make a temporary alloca to pass the argument.
> -      llvm::Value *DestMem =  
> CGF.CreateTempAlloca(ConvertType(ArgTy));
> -      CGF.EmitComplexExprIntoAddr(ArgExpr, DestMem, false);
> -      Args.push_back(DestMem);
> -    } else {
> -      llvm::Value *DestMem =  
> CGF.CreateTempAlloca(ConvertType(ArgTy));
> -      CGF.EmitAggExpr(ArgExpr, DestMem, false);
> -      Args.push_back(DestMem);
> -    }
> -  }
> -
> -  // Get the selector string
> -  std::string SelStr = E->getSelector().getName();
> -  llvm::Constant *Selector = CGF.CGM.GetAddrOfConstantString(SelStr);
> -
> -  llvm::Value *SelPtr = Builder.CreateStructGEP(Selector, 0);
> -  return Runtime->generateMessageSend(Builder, ConvertType(E- 
> >getType()),
> -                                      CGF.LoadObjCSelf(),
> -                                      Receiver, SelPtr,
> -                                      &Args[0], Args.size());
> -}
> -

I guess this is moved somewhere else ?

>  Value  
> *ScalarExprEmitter::VisitArraySubscriptExpr(ArraySubscriptExpr *E) {
>    // Emit subscript expressions in rvalue context's.  For most  
> cases, this just
>    // loads the lvalue formed by the subscript expr.  However, we  
> have to be
> @@ -522,11 +490,11 @@
>      // will not true when we add support for VLAs.
>      Value *V = EmitLValue(Op).getAddress();  // Bitfields can't be  
> arrays.
>
> -    assert(isa<llvm::PointerType>(V->getType()) &&
> -           isa<llvm::ArrayType>(cast<llvm::PointerType>(V->getType())
> -                                ->getElementType()) &&
> -           "Doesn't support VLAs yet!");
> -    V = Builder.CreateStructGEP(V, 0, "arraydecay");
> +    if (isa<llvm::PointerType>(V->getType()) &&
> +        isa<llvm::ArrayType>(cast<llvm::PointerType>(V->getType())
> +          ->getElementType())) {
> +      V = Builder.CreateStructGEP(V, 0, "arraydecay");
> +    }
>
>      // The resultant pointer type can be implicitly casted to other  
> pointer
>      // types as well, for example void*.
> @@ -660,6 +628,13 @@
>    if (TypeToSize->isVoidType())
>      return llvm::ConstantInt::get(llvm::APInt(ResultWidth, 1));
>
> +  // Get the size of VLAs
> +  if (TypeToSize->isVariablyModifiedType() && isSizeOf) {
> +    const VariableArrayType *VLA = TypeToSize- 
> >getAsVariableArrayType();
> +    Value *ElementSize = EmitSizeAlignOf(VLA->getElementType(),  
> RetType, true);
> +    Value *Elements = CGF.EmitScalarExpr(VLA->getSizeExpr());
> +    return Builder.CreateMul(Elements, ElementSize);
> +  }
>    /// FIXME: This doesn't handle VLAs yet!
>    std::pair<uint64_t, unsigned> Info =  
> CGF.getContext().getTypeInfo(TypeToSize);


Please include these with VLA patch so that it is possible to review  
VLA support.

>
> @@ -908,6 +883,11 @@
>                                    LHS, RHS, "cmp");
>      } else {
>        // Signed integers and pointers.
> +      const llvm::Type *LHSTy = LHS->getType();
> +      // Are we comparing pointers to different types?
> +      if (RHS->getType() != LHSTy) {
> +        RHS = Builder.CreateBitCast(RHS, LHSTy);

createPointerCast ?

> @@ -629,9 +637,11 @@

>
>    // Handle struct-return functions by passing a pointer to the  
> location that
>    // we would like to return into.
> +  int RealArgStart = 0;
>    if (hasAggregateLLVMType(ResultType)) {
>      // Create a temporary alloca to hold the result of the call. :(
>      Args.push_back(CreateTempAlloca(ConvertType(ResultType)));
> +    RealArgStart++;
>      // FIXME: set the stret attribute on the argument.
>    }
>
> @@ -640,7 +650,16 @@
>
>      if (!hasAggregateLLVMType(ArgTy)) {
>        // Scalar argument is passed by-value.
> -      Args.push_back(EmitScalarExpr(ArgExprs[i]));
> +      llvm::Value *ArgVal = EmitScalarExpr(ArgExprs[i]);
> +      const llvm::FunctionType *CalleeTy = cast<llvm::FunctionType>(
> +          cast<llvm::PointerType>(Callee->getType())- 
> >getElementType());
> +      if (i < CalleeTy->getNumParams()) {

use assert instead of this check. Otherwise this part is OK.

> +        const llvm::Type *ArgTy = CalleeTy->getParamType(i +  
> RealArgStart);
> +        if (ArgTy && (ArgVal->getType() != ArgTy)) {
> +          ArgVal = Builder.CreateBitCast(ArgVal, ArgTy);
> +        }
> +      }
> +      Args.push_back(ArgVal);
>      } else if (ArgTy->isAnyComplexType()) {
>        // Make a temporary alloca to pass the argument.
>        llvm::Value *DestMem = CreateTempAlloca(ConvertType(ArgTy));

LLVM and clangs development style prefers small and incremental  
changes. We are able to introduce big features and huge improvements  
using this style. If you adhere to this style then it'll be easier for  
you to make progress faster.

-
Devang