[cfe-commits] r117403 - in /cfe/trunk: lib/CodeGen/CGBuiltin.cpp lib/CodeGen/CGExpr.cpp lib/CodeGen/CodeGenFunction.h test/CodeGen/atomic.c

Rafael Espíndola rafael.espindola at gmail.com
Wed Oct 27 10:16:20 PDT 2010


I reverted this as it caused

http://llvm.org/bugs/show_bug.cgi?id=8480


On 26 October 2010 18:09, John McCall <rjmccall at apple.com> wrote:
> Author: rjmccall
> Date: Tue Oct 26 17:09:15 2010
> New Revision: 117403
>
> URL: http://llvm.org/viewvc/llvm-project?rev=117403&view=rev
> Log:
> Extract procedures to do scalar-to-memory and memory-to-scalar conversions
> in IR gen, and use those to fix a correctness issue with bool atomic
> intrinsics.  rdar://problem/8461234
>
>
> Modified:
>    cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>    cfe/trunk/lib/CodeGen/CGExpr.cpp
>    cfe/trunk/lib/CodeGen/CodeGenFunction.h
>    cfe/trunk/test/CodeGen/atomic.c
>
> Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=117403&r1=117402&r2=117403&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Tue Oct 26 17:09:15 2010
> @@ -41,29 +41,28 @@
>                          C, C + 5);
>  }
>
> -static Value *EmitCastToInt(CodeGenFunction &CGF,
> -                            const llvm::Type *ToType, Value *Val) {
> -  if (Val->getType()->isPointerTy())
> -    return CGF.Builder.CreatePtrToInt(Val, ToType);
> -
> -  assert(Val->getType()->isIntegerTy() &&
> -         "Used a non-integer and non-pointer type with atomic builtin");
> -  assert(Val->getType()->getScalarSizeInBits() <=
> -         ToType->getScalarSizeInBits() && "Integer type too small");
> -  return CGF.Builder.CreateSExtOrBitCast(Val, ToType);
> +/// Emit the conversions required to turn the given value into an
> +/// integer of the given size.
> +static Value *EmitToInt(CodeGenFunction &CGF, llvm::Value *V,
> +                        QualType T, const llvm::IntegerType *IntType) {
> +  V = CGF.EmitToMemory(V, T);
> +
> +  if (V->getType()->isPointerTy())
> +    return CGF.Builder.CreatePtrToInt(V, IntType);
> +
> +  assert(V->getType() == IntType);
> +  return V;
>  }
>
> -static Value *EmitCastFromInt(CodeGenFunction &CGF, QualType ToQualType,
> -                              Value *Val) {
> -  const llvm::Type *ToType = CGF.ConvertType(ToQualType);
> -  if (ToType->isPointerTy()) {
> -    return CGF.Builder.CreateIntToPtr(Val, ToType);
> -  }
> -  assert(Val->getType()->isIntegerTy() &&
> -         "Used a non-integer and non-pointer type with atomic builtin");
> -  assert(Val->getType()->getScalarSizeInBits() >=
> -         ToType->getScalarSizeInBits() && "Integer type too small");
> -  return CGF.Builder.CreateTruncOrBitCast(Val, ToType);
> +static Value *EmitFromInt(CodeGenFunction &CGF, llvm::Value *V,
> +                          QualType T, const llvm::Type *ResultType) {
> +  V = CGF.EmitFromMemory(V, T);
> +
> +  if (ResultType->isPointerTy())
> +    return CGF.Builder.CreateIntToPtr(V, ResultType);
> +
> +  assert(V->getType() == ResultType);
> +  return V;
>  }
>
>  // The atomic builtins are also full memory barriers. This is a utility for
> @@ -85,48 +84,69 @@
>  /// and the expression node.
>  static RValue EmitBinaryAtomic(CodeGenFunction &CGF,
>                                Intrinsic::ID Id, const CallExpr *E) {
> +  QualType T = E->getType();
> +  assert(E->getArg(0)->getType()->isPointerType());
> +  assert(CGF.getContext().hasSameUnqualifiedType(T,
> +                                  E->getArg(0)->getType()->getPointeeType()));
> +  assert(CGF.getContext().hasSameUnqualifiedType(T, E->getArg(1)->getType()));
> +
>   llvm::Value *DestPtr = CGF.EmitScalarExpr(E->getArg(0));
>   unsigned AddrSpace =
>     cast<llvm::PointerType>(DestPtr->getType())->getAddressSpace();
> -  const llvm::Type *ValueType =
> +
> +  const llvm::IntegerType *IntType =
>     llvm::IntegerType::get(CGF.getLLVMContext(),
> -                           CGF.getContext().getTypeSize(E->getType()));
> -  const llvm::Type *PtrType = ValueType->getPointerTo(AddrSpace);
> -  const llvm::Type *IntrinsicTypes[2] = { ValueType, PtrType };
> -  Value *AtomF = CGF.CGM.getIntrinsic(Id, IntrinsicTypes, 2);
> -
> -  Value *Args[2] = { CGF.Builder.CreateBitCast(DestPtr, PtrType),
> -                     EmitCastToInt(CGF, ValueType,
> -                                   CGF.EmitScalarExpr(E->getArg(1))) };
> -  return RValue::get(EmitCastFromInt(CGF, E->getType(),
> -                                     EmitCallWithBarrier(CGF, AtomF, Args,
> -                                                         Args + 2)));
> +                           CGF.getContext().getTypeSize(T));
> +  const llvm::Type *IntPtrType = IntType->getPointerTo(AddrSpace);
> +
> +  const llvm::Type *IntrinsicTypes[2] = { IntType, IntPtrType };
> +  llvm::Value *AtomF = CGF.CGM.getIntrinsic(Id, IntrinsicTypes, 2);
> +
> +  llvm::Value *Args[2];
> +  Args[0] = CGF.Builder.CreateBitCast(DestPtr, IntPtrType);
> +  Args[1] = CGF.EmitScalarExpr(E->getArg(1));
> +  const llvm::Type *ValueType = Args[1]->getType();
> +  Args[1] = EmitToInt(CGF, Args[1], T, IntType);
> +
> +  llvm::Value *Result = EmitCallWithBarrier(CGF, AtomF, Args, Args + 2);
> +  Result = EmitFromInt(CGF, Result, T, ValueType);
> +  return RValue::get(Result);
>  }
>
>  /// Utility to insert an atomic instruction based Instrinsic::ID and
> -// the expression node, where the return value is the result of the
> -// operation.
> +/// the expression node, where the return value is the result of the
> +/// operation.
>  static RValue EmitBinaryAtomicPost(CodeGenFunction &CGF,
>                                    Intrinsic::ID Id, const CallExpr *E,
>                                    Instruction::BinaryOps Op) {
> +  QualType T = E->getType();
> +  assert(E->getArg(0)->getType()->isPointerType());
> +  assert(CGF.getContext().hasSameUnqualifiedType(T,
> +                                  E->getArg(0)->getType()->getPointeeType()));
> +  assert(CGF.getContext().hasSameUnqualifiedType(T, E->getArg(1)->getType()));
> +
>   llvm::Value *DestPtr = CGF.EmitScalarExpr(E->getArg(0));
>   unsigned AddrSpace =
>     cast<llvm::PointerType>(DestPtr->getType())->getAddressSpace();
> -
> -  const llvm::Type *ValueType =
> +
> +  const llvm::IntegerType *IntType =
>     llvm::IntegerType::get(CGF.getLLVMContext(),
> -                           CGF.getContext().getTypeSize(E->getType()));
> -  const llvm::Type *PtrType = ValueType->getPointerTo(AddrSpace);
> -  const llvm::Type *IntrinsicTypes[2] = { ValueType, PtrType };
> -  Value *AtomF = CGF.CGM.getIntrinsic(Id, IntrinsicTypes, 2);
> -
> -  Value *Args[2] = { CGF.Builder.CreateBitCast(DestPtr, PtrType),
> -                     EmitCastToInt(CGF, ValueType,
> -                                   CGF.EmitScalarExpr(E->getArg(1))) };
> -  Value *Result = EmitCallWithBarrier(CGF, AtomF, Args, Args + 2);
> -  return RValue::get(EmitCastFromInt(CGF, E->getType(),
> -                                     CGF.Builder.CreateBinOp(Op, Result,
> -                                                             Args[1])));
> +                           CGF.getContext().getTypeSize(T));
> +  const llvm::Type *IntPtrType = IntType->getPointerTo(AddrSpace);
> +
> +  const llvm::Type *IntrinsicTypes[2] = { IntType, IntPtrType };
> +  llvm::Value *AtomF = CGF.CGM.getIntrinsic(Id, IntrinsicTypes, 2);
> +
> +  llvm::Value *Args[2];
> +  Args[1] = CGF.EmitScalarExpr(E->getArg(1));
> +  const llvm::Type *ValueType = Args[1]->getType();
> +  Args[1] = EmitToInt(CGF, Args[1], T, IntType);
> +  Args[0] = CGF.Builder.CreateBitCast(DestPtr, IntPtrType);
> +
> +  llvm::Value *Result = EmitCallWithBarrier(CGF, AtomF, Args, Args + 2);
> +  Result = CGF.Builder.CreateBinOp(Op, Result, Args[1]);
> +  Result = EmitFromInt(CGF, Result, T, ValueType);
> +  return RValue::get(Result);
>  }
>
>  /// EmitFAbs - Emit a call to fabs/fabsf/fabsl, depending on the type of ValTy,
> @@ -793,25 +813,29 @@
>   case Builtin::BI__sync_val_compare_and_swap_4:
>   case Builtin::BI__sync_val_compare_and_swap_8:
>   case Builtin::BI__sync_val_compare_and_swap_16: {
> +    QualType T = E->getType();
>     llvm::Value *DestPtr = CGF.EmitScalarExpr(E->getArg(0));
>     unsigned AddrSpace =
>       cast<llvm::PointerType>(DestPtr->getType())->getAddressSpace();
> -    const llvm::Type *ValueType =
> +
> +    const llvm::IntegerType *IntType =
>       llvm::IntegerType::get(CGF.getLLVMContext(),
> -                             CGF.getContext().getTypeSize(E->getType()));
> -    const llvm::Type *PtrType = ValueType->getPointerTo(AddrSpace);
> -    const llvm::Type *IntrinsicTypes[2] = { ValueType, PtrType };
> +                             CGF.getContext().getTypeSize(T));
> +    const llvm::Type *IntPtrType = IntType->getPointerTo(AddrSpace);
> +    const llvm::Type *IntrinsicTypes[2] = { IntType, IntPtrType };
>     Value *AtomF = CGM.getIntrinsic(Intrinsic::atomic_cmp_swap,
>                                     IntrinsicTypes, 2);
>
> -    Value *Args[3] = { Builder.CreateBitCast(DestPtr, PtrType),
> -                       EmitCastToInt(CGF, ValueType,
> -                                     CGF.EmitScalarExpr(E->getArg(1))),
> -                       EmitCastToInt(CGF, ValueType,
> -                                     CGF.EmitScalarExpr(E->getArg(2))) };
> -    return RValue::get(EmitCastFromInt(CGF, E->getType(),
> -                                       EmitCallWithBarrier(CGF, AtomF, Args,
> -                                                           Args + 3)));
> +    Value *Args[3];
> +    Args[0] = Builder.CreateBitCast(DestPtr, IntPtrType);
> +    Args[1] = CGF.EmitScalarExpr(E->getArg(1));
> +    const llvm::Type *ValueType = Args[1]->getType();
> +    Args[1] = EmitToInt(CGF, Args[1], T, IntType);
> +    Args[2] = EmitToInt(CGF, CGF.EmitScalarExpr(E->getArg(2)), T, IntType);
> +
> +    Value *Result = EmitCallWithBarrier(CGF, AtomF, Args, Args + 3);
> +    Result = EmitFromInt(CGF, Result, T, ValueType);
> +    return RValue::get(Result);
>   }
>
>   case Builtin::BI__sync_bool_compare_and_swap_1:
> @@ -819,27 +843,30 @@
>   case Builtin::BI__sync_bool_compare_and_swap_4:
>   case Builtin::BI__sync_bool_compare_and_swap_8:
>   case Builtin::BI__sync_bool_compare_and_swap_16: {
> +    QualType T = E->getArg(1)->getType();
>     llvm::Value *DestPtr = CGF.EmitScalarExpr(E->getArg(0));
>     unsigned AddrSpace =
>       cast<llvm::PointerType>(DestPtr->getType())->getAddressSpace();
> -    const llvm::Type *ValueType =
> +
> +    const llvm::IntegerType *IntType =
>       llvm::IntegerType::get(CGF.getLLVMContext(),
> -        CGF.getContext().getTypeSize(E->getArg(1)->getType()));
> -    const llvm::Type *PtrType = ValueType->getPointerTo(AddrSpace);
> -    const llvm::Type *IntrinsicTypes[2] = { ValueType, PtrType };
> +                             CGF.getContext().getTypeSize(T));
> +    const llvm::Type *IntPtrType = IntType->getPointerTo(AddrSpace);
> +    const llvm::Type *IntrinsicTypes[2] = { IntType, IntPtrType };
>     Value *AtomF = CGM.getIntrinsic(Intrinsic::atomic_cmp_swap,
>                                     IntrinsicTypes, 2);
>
> -    Value *Args[3] = { Builder.CreateBitCast(DestPtr, PtrType),
> -                       EmitCastToInt(CGF, ValueType,
> -                                     CGF.EmitScalarExpr(E->getArg(1))),
> -                       EmitCastToInt(CGF, ValueType,
> -                                     CGF.EmitScalarExpr(E->getArg(2))) };
> +    Value *Args[3];
> +    Args[0] = Builder.CreateBitCast(DestPtr, IntPtrType);
> +    Args[1] = EmitToInt(CGF, CGF.EmitScalarExpr(E->getArg(1)), T, IntType);
> +    Args[2] = EmitToInt(CGF, CGF.EmitScalarExpr(E->getArg(2)), T, IntType);
> +
>     Value *OldVal = Args[1];
>     Value *PrevVal = EmitCallWithBarrier(*this, AtomF, Args, Args + 3);
>     Value *Result = Builder.CreateICmpEQ(PrevVal, OldVal);
>     // zext bool to int.
> -    return RValue::get(Builder.CreateZExt(Result, ConvertType(E->getType())));
> +    Result = Builder.CreateZExt(Result, ConvertType(E->getType()));
> +    return RValue::get(Result);
>   }
>
>   case Builtin::BI__sync_lock_test_and_set_1:
>
> Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=117403&r1=117402&r2=117403&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Tue Oct 26 17:09:15 2010
> @@ -590,13 +590,7 @@
>   if (TBAAInfo)
>     CGM.DecorateInstruction(Load, TBAAInfo);
>
> -  // Bool can have different representation in memory than in registers.
> -  llvm::Value *V = Load;
> -  if (Ty->isBooleanType())
> -    if (V->getType() != llvm::Type::getInt1Ty(VMContext))
> -      V = Builder.CreateTrunc(V, llvm::Type::getInt1Ty(VMContext), "tobool");
> -
> -  return V;
> +  return EmitFromMemory(Load, Ty);
>  }
>
>  static bool isBooleanUnderlyingType(QualType Ty) {
> @@ -605,17 +599,31 @@
>   return false;
>  }
>
> -void CodeGenFunction::EmitStoreOfScalar(llvm::Value *Value, llvm::Value *Addr,
> -                                        bool Volatile, unsigned Alignment,
> -                                        QualType Ty,
> -                                        llvm::MDNode *TBAAInfo) {
> +llvm::Value *CodeGenFunction::EmitToMemory(llvm::Value *Value, QualType Ty) {
> +  // Bool has a different representation in memory than in registers.
> +  if (Ty->isBooleanType() || isBooleanUnderlyingType(Ty)) {
> +    assert(Value->getType()->isIntegerTy(1) && "value rep of bool not i1");
> +    return Builder.CreateZExt(Value, Builder.getInt8Ty(), "frombool");
> +  }
>
> +  return Value;
> +}
> +
> +llvm::Value *CodeGenFunction::EmitFromMemory(llvm::Value *Value, QualType Ty) {
> +  // Bool has a different representation in memory than in registers.
>   if (Ty->isBooleanType() || isBooleanUnderlyingType(Ty)) {
> -    // Bool can have different representation in memory than in registers.
> -    const llvm::PointerType *DstPtr = cast<llvm::PointerType>(Addr->getType());
> -    Value = Builder.CreateIntCast(Value, DstPtr->getElementType(), false);
> +    assert(Value->getType()->isIntegerTy(8) && "memory rep of bool not i8");
> +    return Builder.CreateTrunc(Value, Builder.getInt1Ty(), "tobool");
>   }
>
> +  return Value;
> +}
> +
> +void CodeGenFunction::EmitStoreOfScalar(llvm::Value *Value, llvm::Value *Addr,
> +                                        bool Volatile, unsigned Alignment,
> +                                        QualType Ty,
> +                                        llvm::MDNode *TBAAInfo) {
> +  Value = EmitToMemory(Value, Ty);
>   llvm::StoreInst *Store = Builder.CreateStore(Value, Addr, Volatile);
>   if (Alignment)
>     Store->setAlignment(Alignment);
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=117403&r1=117402&r2=117403&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Tue Oct 26 17:09:15 2010
> @@ -1349,6 +1349,14 @@
>   /// object.
>   LValue EmitCheckedLValue(const Expr *E);
>
> +  /// EmitToMemory - Change a scalar value from its value
> +  /// representation to its in-memory representation.
> +  llvm::Value *EmitToMemory(llvm::Value *Value, QualType Ty);
> +
> +  /// EmitFromMemory - Change a scalar value from its memory
> +  /// representation to its value representation.
> +  llvm::Value *EmitFromMemory(llvm::Value *Value, QualType Ty);
> +
>   /// EmitLoadOfScalar - Load a scalar value from an address, taking
>   /// care to appropriately convert from the memory representation to
>   /// the LLVM value representation.
>
> Modified: cfe/trunk/test/CodeGen/atomic.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/atomic.c?rev=117403&r1=117402&r2=117403&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGen/atomic.c (original)
> +++ cfe/trunk/test/CodeGen/atomic.c Tue Oct 26 17:09:15 2010
> @@ -102,8 +102,7 @@
>
>   if ( __sync_val_compare_and_swap(&valb, 0, 1)) {
>     // CHECK: call void @llvm.memory.barrier(i1 true, i1 true, i1 true, i1 true, i1 true)
> -// FIXME: Doesn't seem right!
> -    // CHECK: call i8 @llvm.atomic.cmp.swap.i8.p0i8(i8* %valb, i8 0, i8 -1)
> +    // CHECK: call i8 @llvm.atomic.cmp.swap.i8.p0i8(i8* %valb, i8 0, i8 1)
>     // CHECK: call void @llvm.memory.barrier(i1 true, i1 true, i1 true, i1 true, i1 true)
>     old = 42;
>   }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>




More information about the cfe-commits mailing list