On Tue, May 17, 2011 at 9:02 AM, Jakub Jelinek <ja...@redhat.com> wrote:
> Hi!
>
> This patch optimizes using peephole2 __sync_fetch_and_add (x, -N) == N
> and __sync_add_and_fetch (x, N) == 0 by just doing lock {add,sub,inc,dec}
> and testing flags, instead of lock xadd plus comparison.
> The sync_old_add<mode> predicate change makes it possible to optimize
> __sync_add_and_fetch with constant second argument to same
> code as __sync_fetch_and_add.  Doing it in peephole2 has a disadvantage
> though, both that the 3 instructions need to be consecutive and
> e.g. that xadd insn has to be supported by the CPU.  Other alternative
> would be to come up with a new bool builtin that would represent the
> whole __sync_fetch_and_add (x, -N) == N operation (perhaps with dot or space
> in its name to make it inaccessible), try to match it during some folding
> and expand it using special optab.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
> this way?
>
> 2011-05-16  Jakub Jelinek  <ja...@redhat.com>
>
>        PR target/48986
>        * config/i386/sync.md (sync_old_add<mode>): Relax operand 2
>        predicate to allow CONST_INT.
>        (*sync_old_add_cmp<mode>): New insn and peephole2 for it.

OK, but please add a comment explaining why we have matched constraint
with non-matched predicate. These operands are otherwise targets for
cleanups ;)

Also, a comment explaining the purpose of the added peephole would be nice.

IMO, the change to sync_old_add<mode> is also appropriate to release branches.

Thanks,
Uros.

Reply via email to