This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
RE: [PATCH, MIPS] Target flag and build option to disable indexed memory OPs.
Moore, Catherine <Catherine_Moore@mentor.com> writes:
> > -----Original Message-----
> > From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> > owner@gcc.gnu.org] On Behalf Of Matthew Fortune
> > Sent: Monday, January 16, 2017 11:25 AM
> > To: Doug Gilmore <Doug.Gilmore@imgtec.com>; gcc-
> > patches@gcc.gnu.org
> > Cc: Moore, Catherine <Catherine_Moore@mentor.com>
> > Subject: RE: [PATCH, MIPS] Target flag and build option to disable
> > indexed memory OPs.
> >
> > Doug Gilmore <Doug.Gilmore@imgtec.com>
> > > I recently bisected PR78176 to problems introduced with r21650.
> > >
> > > Given the short time until the release, we would like to provide a
> > > target flag and build option to avoid the bug until we are able to
> > > resolve the problem with the commit. Note that as Matthew Fortune
> > has
> > > mentioned in the PR:
> > >
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78176#c5
> > >
> > > the problem could also be addressed by updates to the Linux kernel
> > since
> > > the problem is only exposed by running MIPS 32-bit binaries on 64-
> > bit
> > > kernels.
> > >
> > > Bootstrapped on X86_64, regression tested on X86_64 and MIPS.
> > >
> > > OK to commit?
> >
> > Given this is a generic reference to indexed load/store and the issue
> > could
> > affect any indexed operation then I think it needs to include all of the
> > following as well:
> >
> > /* ISA has lwxs instruction (load w/scaled index address. */
> > #define ISA_HAS_LWXS ((TARGET_SMARTMIPS ||
> > TARGET_MICROMIPS) \
> > && !TARGET_MIPS16)
> >
> > /* ISA has lbx, lbux, lhx, lhx, lhux, lwx, lwux, or ldx instruction. */
> > #define ISA_HAS_LBX (TARGET_OCTEON2)
> > #define ISA_HAS_LBUX (ISA_HAS_DSP || TARGET_OCTEON2)
> > #define ISA_HAS_LHX (ISA_HAS_DSP || TARGET_OCTEON2)
> > #define ISA_HAS_LHUX (TARGET_OCTEON2)
> > #define ISA_HAS_LWX (ISA_HAS_DSP || TARGET_OCTEON2)
> > #define ISA_HAS_LWUX (TARGET_OCTEON2 && TARGET_64BIT)
> > #define ISA_HAS_LDX ((ISA_HAS_DSP || TARGET_OCTEON2) \
> > && TARGET_64BIT)
> >
> > The DSP LBUX/LHX/LWX/LDX intrinsics will also need a new AVAIL
> > predicate
> > to disable them. The snag is that some DSP code will fail to compile if it
> > uses the DSP load intrinsics directly.
> >
> > I see no way of avoiding that. Therefore, distributions that use
> > --without-indexed-load-store will have to cope with some potential
> > DSP
> > fallout if they enable DSP at all.
> >
> > @Catherine: I'd like your input here if possible as I advocated this
> > approach, comments on option names welcome too. I quite like the
> > verbose
> > name.
>
> Okay, based on my reading of the comments in the bug report, you are proposing this option
> as a workaround to a kernel deficiency. I don't see any agreement that this is actually a
> compiler bug.
> Do we really need to include the DSP instrinsics as well? Do you think that many
> distributions actually enable DSP?
>
> The option name itself is acceptable to me. I'd like to see documentation that explains
> when this problem is exposed. I'd like to limit the fix to LWXS and I'd like to see the
> testcase from the bug report added to the testsuite.
> I also agree that the preprocessor macro is a good idea (even if we decide to forgo the
> DSP portion of the patch).
Thanks for the comments.
Having thought further I agree we can safely ignore DSP indexed load and micromips LWXS on
the basis that DSP code will not run on a MIPS64 processor anyway (at least none that I
know of) so the issue cannot occur and similarly for microMIPS, there are no 64-bit cores.
Restricting to just LWXC1/SWXC1/LDXC1/SDXC1 is therefore fine but we should reflect
that in option names then.
--with-lxc1-sxc1 --without-lxc1-sxc1
-mlxc1-sxc1
These names reflect the internal macro that controls availability of these instructions.
Macro name: __mips_no_lxc1_sxc1
Defined when !ISA_HAS_LXC1_SXC1 so would be present even when targeting a core that
doesn't have the instructions anyway.
Any refinements to this Catherine?
Thanks,
Matthew