Improve the representation of the atomic builtins in a few ways. First, we make

their call expressions synthetically have the "deduced" types based on their
first argument. We only insert conversions in the AST for arguments whose
values require conversion to match the value type expected. This keeps PR7600
closed by maintaining the return type, but avoids assertions due to unexpected
implicit casts making the type unsigned (test case added from Daniel).

The magic is moved into the codegen for the atomic builtin which inserts the
casts as needed at the IR level to raise the type to an integer suitable for
the LLVM intrinsic. This shouldn't cause any real change in functionality, but
now we can make the builtin be more truly polymorphic.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@108638 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/CodeGen/CGBuiltin.cpp b/lib/CodeGen/CGBuiltin.cpp
index fff4bacab6b56f48c41f0af26e59c0cfedca6933..0614011932115a39ddccc12a0d27479fdfd172ee 100644 (file)
--- a/lib/CodeGen/CGBuiltin.cpp
+++ b/lib/CodeGen/CGBuiltin.cpp
@@ -41,6 +41,31 @@ static void EmitMemoryBarrier(CodeGenFunction &CGF,
                          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);
+}
+
+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);
+}
+
 // The atomic builtins are also full memory barriers. This is a utility for
 // wrapping a call to the builtins with memory barriers.
 static Value *EmitCallWithBarrier(CodeGenFunction &CGF, Value *Fn,
@@ -60,13 +85,20 @@ static Value *EmitCallWithBarrier(CodeGenFunction &CGF, Value *Fn,
 /// and the expression node.
 static RValue EmitBinaryAtomic(CodeGenFunction &CGF,
                                Intrinsic::ID Id, const CallExpr *E) {
-  Value *Args[2] = { CGF.EmitScalarExpr(E->getArg(0)),
-                     CGF.EmitScalarExpr(E->getArg(1)) };
-  const llvm::Type *ResType[2];
-  ResType[0] = CGF.ConvertType(E->getType());
-  ResType[1] = CGF.ConvertType(E->getArg(0)->getType());
-  Value *AtomF = CGF.CGM.getIntrinsic(Id, ResType, 2);
-  return RValue::get(EmitCallWithBarrier(CGF, AtomF, Args, Args + 2));
+  const llvm::Type *ValueType =
+    llvm::IntegerType::get(CGF.getLLVMContext(),
+                           CGF.getContext().getTypeSize(E->getType()));
+  const llvm::Type *PtrType = ValueType->getPointerTo();
+  const llvm::Type *IntrinsicTypes[2] = { ValueType, PtrType };
+  Value *AtomF = CGF.CGM.getIntrinsic(Id, IntrinsicTypes, 2);
+
+  Value *Args[2] = { CGF.Builder.CreateBitCast(CGF.EmitScalarExpr(E->getArg(0)),
+                                               PtrType),
+                     EmitCastToInt(CGF, ValueType,
+                                   CGF.EmitScalarExpr(E->getArg(1))) };
+  return RValue::get(EmitCastFromInt(CGF, E->getType(),
+                                     EmitCallWithBarrier(CGF, AtomF, Args,
+                                                         Args + 2)));
 }
 
 /// Utility to insert an atomic instruction based Instrinsic::ID and
@@ -75,14 +107,21 @@ static RValue EmitBinaryAtomic(CodeGenFunction &CGF,
 static RValue EmitBinaryAtomicPost(CodeGenFunction &CGF,
                                    Intrinsic::ID Id, const CallExpr *E,
                                    Instruction::BinaryOps Op) {
-  const llvm::Type *ResType[2];
-  ResType[0] = CGF.ConvertType(E->getType());
-  ResType[1] = CGF.ConvertType(E->getArg(0)->getType());
-  Value *AtomF = CGF.CGM.getIntrinsic(Id, ResType, 2);
-  Value *Args[2] = { CGF.EmitScalarExpr(E->getArg(0)),
-                     CGF.EmitScalarExpr(E->getArg(1)) };
+  const llvm::Type *ValueType =
+    llvm::IntegerType::get(CGF.getLLVMContext(),
+                           CGF.getContext().getTypeSize(E->getType()));
+  const llvm::Type *PtrType = ValueType->getPointerTo();
+  const llvm::Type *IntrinsicTypes[2] = { ValueType, PtrType };
+  Value *AtomF = CGF.CGM.getIntrinsic(Id, IntrinsicTypes, 2);
+
+  Value *Args[2] = { CGF.Builder.CreateBitCast(CGF.EmitScalarExpr(E->getArg(0)),
+                                               PtrType),
+                     EmitCastToInt(CGF, ValueType,
+                                   CGF.EmitScalarExpr(E->getArg(1))) };
   Value *Result = EmitCallWithBarrier(CGF, AtomF, Args, Args + 2);
-  return RValue::get(CGF.Builder.CreateBinOp(Op, Result, Args[1]));
+  return RValue::get(EmitCastFromInt(CGF, E->getType(),
+                                     CGF.Builder.CreateBinOp(Op, Result,
+                                                             Args[1])));
 }
 
 /// EmitFAbs - Emit a call to fabs/fabsf/fabsl, depending on the type of ValTy,
@@ -746,14 +785,23 @@ RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD,
   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: {
-    const llvm::Type *ResType[2];
-    ResType[0]= ConvertType(E->getType());
-    ResType[1] = ConvertType(E->getArg(0)->getType());
-    Value *AtomF = CGM.getIntrinsic(Intrinsic::atomic_cmp_swap, ResType, 2);
-    Value *Args[3] = { EmitScalarExpr(E->getArg(0)),
-                       EmitScalarExpr(E->getArg(1)),
-                       EmitScalarExpr(E->getArg(2)) };
-    return RValue::get(EmitCallWithBarrier(*this, AtomF, Args, Args + 3));
+    const llvm::Type *ValueType =
+      llvm::IntegerType::get(CGF.getLLVMContext(),
+                             CGF.getContext().getTypeSize(E->getType()));
+    const llvm::Type *PtrType = ValueType->getPointerTo();
+    const llvm::Type *IntrinsicTypes[2] = { ValueType, PtrType };
+    Value *AtomF = CGM.getIntrinsic(Intrinsic::atomic_cmp_swap,
+                                    IntrinsicTypes, 2);
+
+    Value *Args[3] = { Builder.CreateBitCast(CGF.EmitScalarExpr(E->getArg(0)),
+                                             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)));
   }
 
   case Builtin::BI__sync_bool_compare_and_swap_1:
@@ -761,14 +809,22 @@ RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD,
   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: {
-    const llvm::Type *ResType[2];
-    ResType[0]= ConvertType(E->getArg(1)->getType());
-    ResType[1] = llvm::PointerType::getUnqual(ResType[0]);
-    Value *AtomF = CGM.getIntrinsic(Intrinsic::atomic_cmp_swap, ResType, 2);
-    Value *OldVal = EmitScalarExpr(E->getArg(1));
-    Value *Args[3] = { EmitScalarExpr(E->getArg(0)),
-                       OldVal,
-                       EmitScalarExpr(E->getArg(2)) };
+    const llvm::Type *ValueType =
+      llvm::IntegerType::get(
+        CGF.getLLVMContext(),
+        CGF.getContext().getTypeSize(E->getArg(1)->getType()));
+    const llvm::Type *PtrType = ValueType->getPointerTo();
+    const llvm::Type *IntrinsicTypes[2] = { ValueType, PtrType };
+    Value *AtomF = CGM.getIntrinsic(Intrinsic::atomic_cmp_swap,
+                                    IntrinsicTypes, 2);
+
+    Value *Args[3] = { Builder.CreateBitCast(CGF.EmitScalarExpr(E->getArg(0)),
+                                             PtrType),
+                       EmitCastToInt(CGF, ValueType,
+                                     CGF.EmitScalarExpr(E->getArg(1))),
+                       EmitCastToInt(CGF, ValueType,
+                                     CGF.EmitScalarExpr(E->getArg(2))) };
+    Value *OldVal = Args[1];
     Value *PrevVal = EmitCallWithBarrier(*this, AtomF, Args, Args + 3);
     Value *Result = Builder.CreateICmpEQ(PrevVal, OldVal);
     // zext bool to int.

diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 370d3503298e89b9254780a8bda689d94374d02d..bedea1820362b153a9be2f784e9fb271d005037c 100644 (file)
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -512,19 +512,10 @@ Sema::SemaBuiltinAtomicOverloaded(OwningExprResult TheCallResult) {
   FunctionDecl *NewBuiltinDecl =
     cast<FunctionDecl>(LazilyCreateBuiltin(NewBuiltinII, NewBuiltinID,
                                            TUScope, false, DRE->getLocStart()));
-  const FunctionProtoType *BuiltinFT =
-    NewBuiltinDecl->getType()->getAs<FunctionProtoType>();
 
-  QualType OrigValType = ValType;
-  ValType = BuiltinFT->getArgType(0)->getAs<PointerType>()->getPointeeType();
-
-  // If the first type needs to be converted (e.g. void** -> int*), do it now.
-  if (BuiltinFT->getArgType(0) != FirstArg->getType()) {
-    ImpCastExprToType(FirstArg, BuiltinFT->getArgType(0), CastExpr::CK_BitCast);
-    TheCall->setArg(0, FirstArg);
-  }
-
-  // Next, walk the valid ones promoting to the right type.
+  // The first argument is by definition correct, we use it's type as the type
+  // of the entire operation. Walk the remaining arguments promoting them to
+  // the deduced value type.
   for (unsigned i = 0; i != NumFixed; ++i) {
     Expr *Arg = TheCall->getArg(i+1);
 
@@ -564,28 +555,10 @@ Sema::SemaBuiltinAtomicOverloaded(OwningExprResult TheCallResult) {
   UsualUnaryConversions(PromotedCall);
   TheCall->setCallee(PromotedCall);
 
-  // Change the result type of the call to match the result type of the decl.
-  TheCall->setType(NewBuiltinDecl->getCallResultType());
-
-  // If the value type was converted to an integer when processing the
-  // arguments (e.g. void* -> int), we need to convert the result back.
-  if (!Context.hasSameUnqualifiedType(ValType, OrigValType)) {
-    Expr *E = TheCallResult.takeAs<Expr>();
-
-    assert(ValType->isIntegerType() &&
-           "We always convert atomic operation values to integers.");
-    // FIXME: Handle floating point value type here too.
-    CastExpr::CastKind Kind;
-    if (OrigValType->isIntegerType())
-      Kind = CastExpr::CK_IntegralCast;
-    else if (OrigValType->hasPointerRepresentation())
-      Kind = CastExpr::CK_IntegralToPointer;
-    else
-      llvm_unreachable("Unhandled original value type!");
-
-    ImpCastExprToType(E, OrigValType, Kind);
-    return Owned(E);
-  }
+  // Change the result type of the call to match the original value type. This
+  // is arbitrary, but the codegen for these builtins ins design to handle it
+  // gracefully.
+  TheCall->setType(ValType);
 
   return move(TheCallResult);
 }

diff --git a/test/CodeGen/atomic.c b/test/CodeGen/atomic.c
index aa5aa1507b037f3d53ab8862d6aefd111d6e877b..8b66bfd6600c9d83c37c1a862de5c72c7d352a8e 100644 (file)
--- a/test/CodeGen/atomic.c
+++ b/test/CodeGen/atomic.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 %s -emit-llvm -o - -triple=i686-apple-darwin9 > %t1
-// RUN: grep @llvm.memory.barrier %t1 | count 38
+// RUN: grep @llvm.memory.barrier %t1 | count 40
 // RUN: grep @llvm.atomic.load.add.i32 %t1 | count 3
 // RUN: grep @llvm.atomic.load.sub.i8 %t1 | count 2
 // RUN: grep @llvm.atomic.load.min.i32 %t1
@@ -19,6 +19,7 @@ int atomic(void)
   int old;
   int val = 1;
   char valc = 1;
+  _Bool valb = 0;
   unsigned int uval = 1;
   int cmp = 0;
 
@@ -43,6 +44,9 @@ int atomic(void)
 
   
   __sync_val_compare_and_swap((void **)0, (void *)0, (void *)0);
+  if ( __sync_val_compare_and_swap(&valb, 0, 1)) {
+    old = 42;
+  }
 
   
   __sync_lock_release(&val);

diff --git a/test/Sema/builtins.c b/test/Sema/builtins.c
index c0a2131868e483f53dc5e998d9abf26de809e5a4..64a4b30492c63157a983519456aaab837c328d72 100644 (file)
--- a/test/Sema/builtins.c
+++ b/test/Sema/builtins.c
@@ -44,6 +44,11 @@ void test9(short v) {
 
   // PR7600: Pointers are implicitly casted to integers and back.
   void *old_ptr = __sync_val_compare_and_swap((void**)0, 0, 0);
+
+  // Ensure the return type is correct even when implicit casts are stripped
+  // away. This triggers an assertion while checking the comparison otherwise.
+  if (__sync_fetch_and_add(&old, 1) == 1) {
+  }
 }