Attached is the target independent llvm.atomic.barrier support, as well as alpha and x86 (sse2) support. This matches Chandler's definitions, and the LangRef patch will just restore that. Non-sse2 barrier will be needed, I think it is "lock; mov %esp, %esp", but I'm not sure. Any objections? I'll take a hack at the front end support for __sync_synchronize after this goes in. Andrew -------------- next part -------------- A non-text attachment was scrubbed... Name: barrier.patch Type: text/x-diff Size: 6626 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20080215/fdd767c0/attachment.patch>
On 2/15/08, Andrew Lenharth <andrewl at lenharth.org> wrote:> I'll take a hack at the front end support for > __sync_synchronize after this goes in.This is the gcc side of the patch. Index: gcc/llvm-convert.cpp ==================================================================--- gcc/llvm-convert.cpp (revision 46956) +++ gcc/llvm-convert.cpp (working copy) @@ -4260,6 +4260,15 @@ EmitBlock(new BasicBlock("")); return true; + case BUILT_IN_SYNCHRONIZE: { + Value* C[4]; + C[0] = C[1] = C[2] = C[3] = ConstantInt::get(Type::Int1Ty, 1); + + Builder.CreateCall(Intrinsic::getDeclaration(TheModule, Intrinsic::atomic_membarrier), + C, C + 4); + return true; + } + #if 1 // FIXME: Should handle these GCC extensions eventually. case BUILT_IN_APPLY_ARGS: case BUILT_IN_APPLY:
This looks good to me after a cursory scan, and seems to match what I had worked up with some very welcome better form on the codegen/backend side. =D I'm glad you've had some time to start hacking on this, it may be several more weeks before I get any time to work on these and other LLVM projects. Hopefully someone can chime in on appropriateness of the DAG and target implementations. One question: is "wmb" not actually useful on Alpha? My reading of docs had indicated it provided store-store memory barrier functionality, but I'm far from an expert on the architecture. Again, glad to see some work on this, and glad you had some time! -Chandler Andrew Lenharth wrote:> Attached is the target independent llvm.atomic.barrier support, as > well as alpha and x86 (sse2) support. This matches Chandler's > definitions, and the LangRef patch will just restore that. Non-sse2 > barrier will be needed, I think it is "lock; mov %esp, %esp", but I'm > not sure. > > Any objections? I'll take a hack at the front end support for > __sync_synchronize after this goes in. > > > Andrew > > > ------------------------------------------------------------------------ > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
On 2/15/08, Chandler Carruth <chandlerc at gmail.com> wrote:> Hopefully someone can chime in on appropriateness of the DAG and target > implementations. One question: is "wmb" not actually useful on Alpha? My > reading of docs had indicated it provided store-store memory barrier > functionality, but I'm far from an expert on the architecture.wmb is probably useful and can be added should someone really want it fairly easily, but since alpha isn't much used, I don't mind being overly conservative there. The bigger problem is the pre-SSE2 x86 version.> Again, glad to see some work on this, and glad you had some time!We have similar extensions sitting around for some research projects and I wanted to reduce our code base :) Andrew
On Feb 15, 2008, at 2:29 PM, Andrew Lenharth wrote:> On 2/15/08, Andrew Lenharth <andrewl at lenharth.org> wrote: >> I'll take a hack at the front end support for >> __sync_synchronize after this goes in. > > This is the gcc side of the patch.GCC 4.2 compiles this to a no-op on x86: void foo() { __sync_synchronize(); } Are you seeing different behavior? What am I missing here? -Chris> > > Index: gcc/llvm-convert.cpp > ==================================================================> --- gcc/llvm-convert.cpp (revision 46956) > +++ gcc/llvm-convert.cpp (working copy) > @@ -4260,6 +4260,15 @@ > EmitBlock(new BasicBlock("")); > return true; > > + case BUILT_IN_SYNCHRONIZE: { > + Value* C[4]; > + C[0] = C[1] = C[2] = C[3] = ConstantInt::get(Type::Int1Ty, 1); > + > + Builder.CreateCall(Intrinsic::getDeclaration(TheModule, > Intrinsic::atomic_membarrier), > + C, C + 4); > + return true; > + } > + > #if 1 // FIXME: Should handle these GCC extensions eventually. > case BUILT_IN_APPLY_ARGS: > case BUILT_IN_APPLY: > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
On Feb 15, 2008, at 2:29 PM, Andrew Lenharth wrote:> On 2/15/08, Andrew Lenharth <andrewl at lenharth.org> wrote: >> I'll take a hack at the front end support for >> __sync_synchronize after this goes in. > > This is the gcc side of the patch.Thanks for tackling this Andrew. Please prepare a patch for LangRef.html that explains what this thing does :). What are all those bools? Once that is available I'll review the rest of the llvm patch. In the call below, you don't have to pass in 4 i1's. Just passing in the intrinsic ID should be fine. The type list is for intrinsics that take 'any' as an argument. Finally, don't forget the 80 column rule. -Chris> > > Index: gcc/llvm-convert.cpp > ==================================================================> --- gcc/llvm-convert.cpp (revision 46956) > +++ gcc/llvm-convert.cpp (working copy) > @@ -4260,6 +4260,15 @@ > EmitBlock(new BasicBlock("")); > return true; > > + case BUILT_IN_SYNCHRONIZE: { > + Value* C[4]; > + C[0] = C[1] = C[2] = C[3] = ConstantInt::get(Type::Int1Ty, 1); > + > + Builder.CreateCall(Intrinsic::getDeclaration(TheModule, > Intrinsic::atomic_membarrier), > + C, C + 4); > + return true; > + } > + > #if 1 // FIXME: Should handle these GCC extensions eventually. > case BUILT_IN_APPLY_ARGS: > case BUILT_IN_APPLY: > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
On Feb 15, 2008, at 1:34 PM, Andrew Lenharth wrote:> Attached is the target independent llvm.atomic.barrier support, as > well as alpha and x86 (sse2) support. This matches Chandler's > definitions, and the LangRef patch will just restore that. Non-sse2 > barrier will be needed, I think it is "lock; mov %esp, %esp", but I'm > not sure. > > Any objections? I'll take a hack at the front end support for > __sync_synchronize after this goes in.+ // MEMBARRIER - This corresponds to the atomic.barrier intrinsic. + // it takes 4 operands to specify the type of barrier: + // ll, ls, sl, ss + MEMBARRIER, This should specify the input and output values, like EH_RETURN does. +def : Pat<(membarrier (i8 1), (i8 0), (i8 0), (i8 0)), (LFENCE)>; +def : Pat<(membarrier (i8 imm:$ll), (i8 imm:$ls), (i8 imm:$sl), (i8 imm:$ss)), + (MFENCE)>; Very nice. I'll trust Chandler that these are right. :) Looks great Andrew, please commit. Thanks! -Chris