[cfe-commits] r108638 - in /cfe/trunk: lib/CodeGen/CGBuiltin.cpp lib/Sema/SemaChecking.cpp test/CodeGen/atomic.c test/Sema/builtins.c

Daniel Dunbar daniel at zuster.org
Sun Jul 18 10:25:06 PDT 2010


Hi Chandler,

I am still running into problems after this patch. The following test
case is crashing in IRgen:
--
ddunbar at lordcrumb:tmp$ cat t.c
void f0(void) {
  void *a, *b;
  __sync_bool_compare_and_swap(&b, 0, a);
}
ddunbar at lordcrumb:tmp$ clang -c t.c
Assertion failed: (castIsValid(getOpcode(), S, Ty) && "Illegal ZExt"),
function ZExtInst, file
/Users/ddunbar/llvm/lib/VMCore/Instructions.cpp, line 2535.
0  clang             0x000000010164ee75 PrintStackTrace(void*) + 53
1  clang             0x000000010164f393 SignalHandler(int) + 371
2  libSystem.B.dylib 0x00007fff8033e35a _sigtramp + 26
3  clang             0x0000000100ae2691
llvm::isa_impl<llvm::Instruction, llvm::Value>::doit(llvm::Value
const&) + 17
4  libSystem.B.dylib 0x00007fff803b99b4 __pthread_markcancel + 0
5  clang             0x000000010155e344
llvm::ZExtInst::ZExtInst(llvm::Value*, llvm::Type const*, llvm::Twine
const&, llvm::Instruction*) + 212
6  clang             0x000000010155bed5
llvm::ZExtInst::ZExtInst(llvm::Value*, llvm::Type const*, llvm::Twine
const&, llvm::Instruction*) + 53
7  clang             0x000000010155bbe1
llvm::CastInst::Create(llvm::Instruction::CastOps, llvm::Value*,
llvm::Type const*, llvm::Twine const&, llvm::Instruction*) + 161
8  clang             0x00000001014c0b17 llvm::IRBuilder<true,
llvm::ConstantFolder, llvm::IRBuilderDefaultInserter<true>
>::CreateCast(llvm::Instruction::CastOps, llvm::Value*, llvm::Type
const*, llvm::Twine const&) + 199
9  clang             0x00000001012478aa llvm::IRBuilder<true,
llvm::ConstantFolder, llvm::IRBuilderDefaultInserter<true>
>::CreateZExt(llvm::Value*, llvm::Type const*, llvm::Twine const&) +
58
10 clang             0x0000000100208468
clang::CodeGen::CodeGenFunction::EmitBuiltinExpr(clang::FunctionDecl
const*, unsigned int, clang::CallExpr const*) + 20248
11 clang             0x000000010025a6a4
clang::CodeGen::CodeGenFunction::EmitCallExpr(clang::CallExpr const*,
clang::CodeGen::ReturnValueSlot) + 388
12 clang             0x0000000100284d98 (anonymous
namespace)::ScalarExprEmitter::VisitCallExpr(clang::CallExpr const*) +
152
13 clang             0x000000010027c928 clang::StmtVisitor<(anonymous
namespace)::ScalarExprEmitter, llvm::Value*>::Visit(clang::Stmt*) +
2024
14 clang             0x000000010027c0f5
clang::CodeGen::CodeGenFunction::EmitScalarExpr(clang::Expr const*,
bool) + 213
15 clang             0x000000010024f4b7
clang::CodeGen::CodeGenFunction::EmitAnyExpr(clang::Expr const*,
llvm::Value*, bool, bool, bool) + 151
16 clang             0x00000001002dc8a4
clang::CodeGen::CodeGenFunction::EmitStmt(clang::Stmt const*) + 484
17 clang             0x00000001002e0704
clang::CodeGen::CodeGenFunction::EmitCompoundStmt(clang::CompoundStmt
const&, bool, llvm::Value*, bool) + 452
18 clang             0x00000001002dcbd1
clang::CodeGen::CodeGenFunction::EmitSimpleStmt(clang::Stmt const*) +
209
19 clang             0x00000001002dc731
clang::CodeGen::CodeGenFunction::EmitStmt(clang::Stmt const*) + 113
20 clang             0x0000000100311162
clang::CodeGen::CodeGenFunction::EmitFunctionBody(llvm::SmallVector<std::pair<clang::VarDecl
const*, clang::QualType>, 16u>&) + 178
21 clang             0x00000001003117ef
clang::CodeGen::CodeGenFunction::GenerateCode(clang::CodeGen::GlobalDecl,
llvm::Function*) + 1663
22 clang             0x000000010031dd67
clang::CodeGen::CodeGenModule::EmitGlobalFunctionDefinition(clang::CodeGen::GlobalDecl)
+ 935
23 clang             0x000000010031b8db
clang::CodeGen::CodeGenModule::EmitGlobalDefinition(clang::CodeGen::GlobalDecl)
+ 619
24 clang             0x000000010031d36f
clang::CodeGen::CodeGenModule::EmitGlobal(clang::CodeGen::GlobalDecl)
+ 991
25 clang             0x00000001003216e2
clang::CodeGen::CodeGenModule::EmitTopLevelDecl(clang::Decl*) + 258
26 clang             0x000000010034de36 (anonymous
namespace)::CodeGeneratorImpl::HandleTopLevelDecl(clang::DeclGroupRef)
+ 102
27 clang             0x000000010030c94f (anonymous
namespace)::BackendConsumer::HandleTopLevelDecl(clang::DeclGroupRef) +
207
28 clang             0x0000000100365a41
clang::ParseAST(clang::Preprocessor&, clang::ASTConsumer*,
clang::ASTContext&, bool, bool, clang::CodeCompleteConsumer*) + 561
29 clang             0x00000001000d0d39
clang::ASTFrontendAction::ExecuteAction() + 297
30 clang             0x000000010030c152
clang::CodeGenAction::ExecuteAction() + 1106
31 clang             0x00000001000d0920 clang::FrontendAction::Execute() + 304
32 clang             0x00000001000a8b98
clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) + 1000
33 clang             0x0000000100048a0f cc1_main(char const**, char
const**, char const*, void*) + 2047
34 clang             0x0000000100055ae1 main + 289
35 clang             0x0000000100048200 start + 52
Stack dump:
0.      Program arguments:
/Volumes/Data/ddunbar/llvm.obj.64/Debug+Asserts/bin/clang -cc1 -triple
x86_64-apple-darwin10.0.0 -emit-obj -mrelax-all -disable-free
-main-file-name t.c -pic-level 1 -mdisable-fp-elim -masm-verbose
-munwind-tables -target-cpu core2 -resource-dir
/Volumes/Data/ddunbar/llvm.obj.64/Debug+A\
sserts/lib/clang/2.8 -ferror-limit 19 -fmessage-length 90
-stack-protector 1 -fblocks -fdiagnostics-show-option -o t.o -x c t.c
1.      <eof> parser at end of file
2.      t.c:1:6: LLVM IR generation of declaration 'f0'
3.      t.c:1:6: Generating code for declaration 'f0'
4.      t.c:1:15: LLVM IR generation of compound statement ('{}')
clang: error: clang frontend command failed due to signal 6 (use -v to
see invocation)
ddunbar at lordcrumb:tmp$
--

 - Daniel

On Sun, Jul 18, 2010 at 12:23 AM, Chandler Carruth <chandlerc at gmail.com> wrote:
> Author: chandlerc
> Date: Sun Jul 18 02:23:17 2010
> New Revision: 108638
>
> URL: http://llvm.org/viewvc/llvm-project?rev=108638&view=rev
> Log:
> 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.
>
> Modified:
>    cfe/trunk/lib/CodeGen/CGBuiltin.cpp
>    cfe/trunk/lib/Sema/SemaChecking.cpp
>    cfe/trunk/test/CodeGen/atomic.c
>    cfe/trunk/test/Sema/builtins.c
>
> Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=108638&r1=108637&r2=108638&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Sun Jul 18 02:23:17 2010
> @@ -41,6 +41,31 @@
>                          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 @@
>  /// 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 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 @@
>   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 @@
>   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.
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=108638&r1=108637&r2=108638&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Sun Jul 18 02:23:17 2010
> @@ -512,19 +512,10 @@
>   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 @@
>   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);
>  }
>
> Modified: cfe/trunk/test/CodeGen/atomic.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/atomic.c?rev=108638&r1=108637&r2=108638&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGen/atomic.c (original)
> +++ cfe/trunk/test/CodeGen/atomic.c Sun Jul 18 02:23:17 2010
> @@ -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 old;
>   int val = 1;
>   char valc = 1;
> +  _Bool valb = 0;
>   unsigned int uval = 1;
>   int cmp = 0;
>
> @@ -43,6 +44,9 @@
>
>
>   __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);
>
> Modified: cfe/trunk/test/Sema/builtins.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtins.c?rev=108638&r1=108637&r2=108638&view=diff
> ==============================================================================
> --- cfe/trunk/test/Sema/builtins.c (original)
> +++ cfe/trunk/test/Sema/builtins.c Sun Jul 18 02:23:17 2010
> @@ -44,6 +44,11 @@
>
>   // 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) {
> +  }
>  }
>
>
>
>
> _______________________________________________
> 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