Moore, Catherine <catherine_mo...@mentor.com> writes: > > -----Original Message----- > > From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- > > ow...@gcc.gnu.org] On Behalf Of Matthew Fortune > > Sent: Monday, January 16, 2017 11:25 AM > > To: Doug Gilmore <doug.gilm...@imgtec.com>; gcc- > > patc...@gcc.gnu.org > > Cc: Moore, Catherine <catherine_mo...@mentor.com> > > Subject: RE: [PATCH, MIPS] Target flag and build option to disable > > indexed memory OPs. > > > > Doug Gilmore <doug.gilm...@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