Re: [PATCH][ARM] PR rtl-optimization/69904: Disallow copying/duplicating of load-exclusive operations

[Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>]

On 03/03/16 09:42, Ramana Radhakrishnan wrote:
> On 03/03/16 09:35, Kyrill Tkachov wrote:
> > Hi all,
> >
> > In this PR shrink-wrapping ends up duplicating the load-exclusive part of a load-exclusive/store-exclusive loop
> > used to implement an atomic compare exchange operation. Look in bugzilla for the kind of sequences generated.
> > The load-exclusive paths are duplicated, but not the store-exclusive ones, with some very counter-intuitive
> > control flow between them.
> >
> > The suggested solution is to catch the load exclusive instructions in TARGET_CANNOT_COPY_INSN_P so that
> > the RTL optimisers don't do any funny tricks with them. Since these instructions only really appear when
> > implementing atomic operations it's unlikely that the user will want particularly fancy transformations involving
> > duplication of exclusive access sequences anyway (though examples to the contrary would certainly be appreciated)
> > and this fix seems like the safest at this point. This is a regression fix from GCC 5 as shrink-wrapping got more
> > aggressive for GCC 6.
> >
> > With this patch the sequence is not shrink-wrapped and is the same as the sequence for GCC 5 as shown in the PR.
> >
> > Bootstrapped and tested on arm-none-linux-gnueabihf.
> >
> > Ok for trunk?
> While reading the patch again I wondered if we should also catch store exclusives here but the cases we are interested in preventing are copies of blocks containing ldrex instructions which almost always are paired with an strex instruction i.e. I don't think an strex instruction can exist without a corresponding ldrex instruction. Thus your patch looks sufficient.

I had considered that too and came to the same conclusion.

> It would also be good to audit the aarch64 backend for any such sequences but that need not hold up this patch.
>
> Thus, Ok for trunk.

Thanks, I'll commit shortly.

Kyrill

> Thanks,
> Ramana
>
> > Thanks,
> > Kyrill
> >
> >
> > 2016-03-03  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> >
> >      PR rtl-optimization/69904
> >      * config/arm/arm.c (arm_cannot_copy_insn_p):
> >      Return true for load-exclusive instructions.
> >
> > 2016-03-03  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> >
> >      PR rtl-optimization/69904
> >      * gcc.target/arm/pr69904.c: New test.