This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [C11-atomic] [patch] gimple atomic statements


On Tue, Apr 3, 2012 at 9:40 PM, Andrew MacLeod <amacleod@redhat.com> wrote:
> Here is my first step in promoting the __atomic builtins into gimple
> statements. ?I originally planned this as tree codes, but when prototyped it
> became obvious that a gimple statement was a far superior solution since we
> also need to deal with LHS and memory address issues.
>
> Motivations are many-fold, but primarily it makes manipulating atomics
> easier and exposes more of their side effects to the optimizers. ?In
> particular we can now expose both return values of the compare and swap when
> implementing compare_exchange and get more efficient code generation. (soon
> :-) ? It is item number 3 on my 4.8 task list:
> http://gcc.gnu.org/wiki/Atomic/GCCMM/gcc4.8.
>
> This first step adds a GIMPLE_ATOMIC statement class which handles all the
> __atomic built-in calls. Right after the cfg is built, all built-in __atomic
> calls are converted to gimple_atomic statements. ?I considered doing the
> conversion right in the gimplifier, but elected to leave it as a pass for
> the time being. ?This then ?passes through all the optimizers to cfgexpand,
> where they are then converted directly into rtl.
>
> This currently ?produces the same code that the builtins do in 4.7. ?I
> expect that I missed a few places in the optimizers where they aren't
> properly treated as barriers yet, ?but I'll get to tracking those down in a
> bit.
>
> I also have not implemented non-integral atomics yet, nor do I issuing
> library calls when inline expansion cannot be done. ?That's next... I just
> want to get the basics checked into the branch.
>
> I expect to be able to wrap the __sync routines into this as well,
> eliminating all the atomic and sync builtin expansion code, keeping
> everything in one easy statement class. ? Then I'll add the _Atomic type
> qualifier to the parser, and have that simply translate expressions
> involving those types into gimple_atomic statements at the same time calls
> are converted.
>
> This bootstraps on x86_64-unknown-linux-gnu, and the only testsuite
> regressions are one involving issuing library calls. ?There is a toolchain
> build problem with libjava however... During libjava construction there ends
> up being files which cant be created due to permission problems in .svn
> directories ... ? Pretty darn weird, but I'll look into it later when the
> atomic gimple support is complete, if the problem it still exists then.
>
> Anyone see anything obviously flawed about the approach?

The fact that you need to touch every place that wants to look at memory
accesses shows that you are doing it wrong.  Instead my plan was to
force _all_ memory accesses to GIMPLE_ASSIGNs (yes, including those
we have now in calls).  You're making a backwards step in my eyes.

What do you think is "easier" when you use a GIMPLE_ATOMIC
(why do you need a fntype field?!  Should the type not be available
via the operand types?!)

Your tree-cfg.c parts need to be filled in.  They are the specification of
GIMPLE_ATOMIC - at the moment you allow any garbage.

Similar to how I dislike the choice of adding GIMPLE_TRANSACTION
instead of using builtin functions I dislike this.

I suppose you do not want to use builtins because for primitive types you
end up with multiple statements for something "atomic"?

So, please tell us why the current scheme does not work and how the
new scheme overcomes this (that's entirely missing in your proposal...)

Thanks,
Richard.

> Andrew
>
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]