[AArch64] Backporting -moutline-atomics to gcc 9.x and 8.x
Christophe Lyon
christophe.lyon@linaro.org
Wed Apr 1 22:13:08 GMT 2020
On Wed, 25 Mar 2020 at 01:24, Pop, Sebastian via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi Kyrill,
>
> Thanks for pointing out the two missing bug fixes.
> Please see attached all the back-ported patches.
> All the patches from trunk applied cleanly with no conflicts (except for the ChangeLog files) to the gcc-9 branch.
> An up to date gcc-9 branch on which I applied the attached patches has passed bootstrap on aarch64-linux (Graviton2 with 64 N1 cores) and make check with no extra fails.
> Kyrill, could you please commit the attached patches to the gcc-9 branch?
>
Hi,
I'm seeing a GCC build failure after "aarch64: Implement TImode
compare-and-swap"
was backported to gcc-9 (commit 53c1356515ac1357c341b594326967ac4677d891)
The build log has:
0x14a1660 gen_split_100(rtx_insn*, rtx_def**)
/tmp/6477245_1.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/config/aarch64/atomics.md:110
0xa81076 try_split(rtx_def*, rtx_insn*, int)
/tmp/6477245_1.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/emit-rtl.c:3851
0xda2b0d split_insn
/tmp/6477245_1.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/recog.c:2901
0xda7057 split_all_insns()
/tmp/6477245_1.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/recog.c:3005
0xda7118 execute
/tmp/6477245_1.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/recog.c:3957
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
make[4]: *** [Makefile:659: tsan_interface_atomic.lo] Error 1
Maybe that problem is fixed by a patch later in the series? (I have
validations running after every patch on the release branches, so it
may take a while until I have the results for the end of the series)
Thanks,
Christophe
> As we still don't have a copyright assignment on file, would it be possible for ARM to finish the backport to the gcc-8 branch of these patches and the atomics cleanup patches mentioned below?
>
> I did a `git log config/aarch64/atomics.md` and there is a follow-up patch to the atomics cleanup patches:
>
> commit e21679a8bb17aac603b8704891e60ac502200629
> Author: Jakub Jelinek <jakub@redhat.com>
> Date: Wed Nov 21 17:41:03 2018 +0100
>
> re PR target/87839 (ICE in final_scan_insn_1, at final.c:3070)
>
> PR target/87839
> * config/aarch64/atomics.md (@aarch64_compare_and_swap<mode>): Use
> rIJ constraint for aarch64_plus_operand rather than rn.
>
> * gcc.target/aarch64/pr87839.c: New test.
>
> From-SVN: r266346
>
> That is fixing code modified in this cleanup patch:
>
> commit d400fda3a8c3330f77eb9d51874f5482d3819a9f
> Author: Richard Henderson <richard.henderson@linaro.org>
> Date: Wed Oct 31 09:42:39 2018 +0000
>
> aarch64: Improve cas generation
>
>
> Thanks,
> Sebastian
>
>
> On 3/11/20, 5:11 AM, "Kyrill Tkachov" <kyrylo.tkachov@foss.arm.com> wrote:
>
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
>
>
>
> Hi Sebastian,
>
> On 3/9/20 9:47 PM, Pop, Sebastian wrote:
> > Hi,
> >
> > Please see attached the patches to add -moutline-atomics to the gcc-9 branch.
> > Tested on graviton2 aarch64-linux with bootstrap and
> > `make check` passes with no new fails.
> > Tested `make check` on glibc built with gcc-9 with and without "-moutline-atomics"
> > and CFLAGS=" -O2 -g -fno-stack-protector -U_FORTIFY_SOURCE".
> >
> > Ok to commit to gcc-9 branch?
>
> Since this feature enables backwards-compatible deployment of LSE
> atomics, I'd support that.
>
> That is okay with me in principle after GCC 9.3 is released (the branch
> is currently frozen).
>
> However, there have been a few follow-up patches to fix some bugs
> revealed by testing.
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91833
>
> and
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91834
>
> come to mind.
>
> Can you please make sure the fixes for those are included as well?
>
>
> >
> > Does this mechanical `git am *.patch` require a copyright assignment?
> > I am still working with my employer on getting the FSF assignment signed.
> >
> > Thanks,
> > Sebastian
> >
> > PS: For gcc-8 backports there are 5 cleanup and improvement patches
> > that are needed for -moutline-atomics patches to apply cleanly.
> > Should these patches be back-ported in the same time as the flag patches,
> > or should I update the patches to apply to the older code base?
>
> Hmm... normally I'd be for them. In this case I'd want to make sure that
> there aren't any fallout fixes that we're missing.
>
> Did these patches have any bug reports against them?
>
> Thanks,
>
> Kyrill
>
>
> > Here is the list of the extra patches:
> >
> > From 77f33f44baf24c22848197aa80962c003dd7b3e2 Mon Sep 17 00:00:00 2001
> > From: Richard Henderson <richard.henderson@linaro.org>
> > Date: Wed, 31 Oct 2018 09:29:29 +0000
> > Subject: [PATCH] aarch64: Simplify LSE cas generation
> >
> > The cas insn is a single insn, and if expanded properly need not
> > be split after reload. Use the proper inputs for the insn.
> >
> > * config/aarch64/aarch64.c (aarch64_expand_compare_and_swap):
> > Force oldval into the rval register for TARGET_LSE; emit the compare
> > during initial expansion so that it may be deleted if unused.
> > (aarch64_gen_atomic_cas): Remove.
> > * config/aarch64/atomics.md (@aarch64_compare_and_swap<SHORT>_lse):
> > Change =&r to +r for operand 0; use match_dup for operand 2;
> > remove is_weak and mod_f operands as unused. Drop the split
> > and merge with...
> > (@aarch64_atomic_cas<SHORT>): ... this pattern's output; remove.
> > (@aarch64_compare_and_swap<GPI>_lse): Similarly.
> > (@aarch64_atomic_cas<GPI>): Similarly.
> >
> > From-SVN: r265656
> >
> > From d400fda3a8c3330f77eb9d51874f5482d3819a9f Mon Sep 17 00:00:00 2001
> > From: Richard Henderson <richard.henderson@linaro.org>
> > Date: Wed, 31 Oct 2018 09:42:39 +0000
> > Subject: [PATCH] aarch64: Improve cas generation
> >
> > Do not zero-extend the input to the cas for subword operations;
> > instead, use the appropriate zero-extending compare insns.
> > Correct the predicates and constraints for immediate expected operand.
> >
> > * config/aarch64/aarch64.c (aarch64_gen_compare_reg_maybe_ze): New.
> > (aarch64_split_compare_and_swap): Use it.
> > (aarch64_expand_compare_and_swap): Likewise. Remove convert_modes;
> > test oldval against the proper predicate.
> > * config/aarch64/atomics.md (@atomic_compare_and_swap<ALLI>):
> > Use nonmemory_operand for expected.
> > (cas_short_expected_pred): New.
> > (@aarch64_compare_and_swap<SHORT>): Use it; use "rn" not "rI" to match.
> > (@aarch64_compare_and_swap<GPI>): Use "rn" not "rI" for expected.
> > * config/aarch64/predicates.md (aarch64_plushi_immediate): New.
> > (aarch64_plushi_operand): New.
> >
> > From-SVN: r265657
> >
> > From 8f5603d363a4e0453d2c38c7103aeb0bdca85c4e Mon Sep 17 00:00:00 2001
> > From: Richard Henderson <richard.henderson@linaro.org>
> > Date: Wed, 31 Oct 2018 09:47:21 +0000
> > Subject: [PATCH] aarch64: Improve swp generation
> >
> > Allow zero as an input; fix constraints; avoid unnecessary split.
> >
> > * config/aarch64/aarch64.c (aarch64_emit_atomic_swap): Remove.
> > (aarch64_gen_atomic_ldop): Don't call it.
> > * config/aarch64/atomics.md (atomic_exchange<ALLI>):
> > Use aarch64_reg_or_zero.
> > (aarch64_atomic_exchange<ALLI>): Likewise.
> > (aarch64_atomic_exchange<ALLI>_lse): Remove split; remove & from
> > operand 0; use aarch64_reg_or_zero for input; merge ...
> > (@aarch64_atomic_swp<ALLI>): ... this and remove.
> >
> > From-SVN: r265659
> >
> > From 7803ec5ee2a547043fb6708a08ddb1361ba91202 Mon Sep 17 00:00:00 2001
> > From: Richard Henderson <richard.henderson@linaro.org>
> > Date: Wed, 31 Oct 2018 09:58:48 +0000
> > Subject: [PATCH] aarch64: Improve atomic-op lse generation
> >
> > Fix constraints; avoid unnecessary split. Drop the use of the atomic_op
> > iterator in favor of the ATOMIC_LDOP iterator; this is simplier and more
> > logical for ldclr aka bic.
> >
> > * config/aarch64/aarch64.c (aarch64_emit_bic): Remove.
> > (aarch64_atomic_ldop_supported_p): Remove.
> > (aarch64_gen_atomic_ldop): Remove.
> > * config/aarch64/atomic.md (atomic_<atomic_optab><ALLI>):
> > Fully expand LSE operations here.
> > (atomic_fetch_<atomic_optab><ALLI>): Likewise.
> > (atomic_<atomic_optab>_fetch<ALLI>): Likewise.
> > (aarch64_atomic_<ATOMIC_LDOP><ALLI>_lse): Drop atomic_op iterator
> > and use ATOMIC_LDOP instead; use register_operand for the input;
> > drop the split and emit insns directly.
> > (aarch64_atomic_fetch_<ATOMIC_LDOP><ALLI>_lse): Likewise.
> > (aarch64_atomic_<atomic_op>_fetch<ALLI>_lse): Remove.
> > (@aarch64_atomic_load<ATOMIC_LDOP><ALLI>): Remove.
> >
> > From-SVN: r265660
> >
> > From 53de1ea800db54b47290d578c43892799b66c8dc Mon Sep 17 00:00:00 2001
> > From: Richard Henderson <richard.henderson@linaro.org>
> > Date: Wed, 31 Oct 2018 23:11:22 +0000
> > Subject: [PATCH] aarch64: Remove early clobber from ATOMIC_LDOP scratch
> >
> > * config/aarch64/atomics.md (aarch64_atomic_<ATOMIC_LDOP><ALLI>_lse):
> > The scratch register need not be early-clobber. Document the reason
> > why we cannot use ST<OP>.
> >
> > From-SVN: r265703
> >
> >
> >
> >
> >
> > On 2/27/20, 12:06 PM, "Kyrill Tkachov" <kyrylo.tkachov@foss.arm.com> wrote:
> >
> > Hi Sebastian,
> >
> > On 2/27/20 4:53 PM, Pop, Sebastian wrote:
> > >
> > > Hi,
> > >
> > > is somebody already working on backporting -moutline-atomics to gcc
> > > 8.x and 9.x branches?
> > >
> > I'm not aware of such work going on.
> >
> > Thanks,
> >
> > Kyrill
> >
> > > Thanks,
> > >
> > > Sebastian
> > >
> >
> >
>
>
More information about the Gcc-patches
mailing list