This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, cleanup] Remove PowerPC -mupper-regs-* options
- From: Segher Boessenkool <segher at kernel dot crashing dot org>
- To: Michael Meissner <meissner at linux dot vnet dot ibm dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, David Edelsohn <dje dot gcc at gmail dot com>, Bill Schmidt <wschmidt at linux dot vnet dot ibm dot com>
- Date: Mon, 24 Jul 2017 05:21:15 -0500
- Subject: Re: [PATCH, cleanup] Remove PowerPC -mupper-regs-* options
- Authentication-results: sourceware.org; auth=none
- References: <20170722064604.GA19115@ibm-tiger.the-meissners.org>
Hi Mike,
On Sat, Jul 22, 2017 at 02:46:04AM -0400, Michael Meissner wrote:
> One thing that I did was continue to define __UPPER_REGS_{DF,SF,DI}__ that were
> previously defined. I can delete them if desired and perhaps poison the names
> so that any use if flaged.
I don't think anything uses it. Certainly nothing *should* use it: it
isn't documented anywhere :-) (Google doesn't find anything either, fwiw).
You can delete them I think. Poisoning is overkill.
> Future patches will include removal of the TARGET_UPPER_REGS_* macros in the
> various files. I also plan to remove -mvsx-small-integer, and the various
> -mpower9-dform* options.
Yay! Thank you.
> Is this ok for trunk? At present, I do not intend to back port this to GCC 7
> (but if the maintainers want that, I can do it).
I don't think that would help much, so yeah, let's not.
> @@ -58,7 +56,6 @@
> | OPTION_MASK_HTM \
> | OPTION_MASK_QUAD_MEMORY \
> | OPTION_MASK_QUAD_MEMORY_ATOMIC \
> - | OPTION_MASK_UPPER_REGS_SF \
> | OPTION_MASK_VSX_SMALL_INTEGER)
>
> /* Add ISEL back into ISA 3.0, since it is supposed to be a win. Do not add
The OPTION_MASK_QUAD_MEMORY_ATOMIC line has errant leading spaces, maybe
you could fix that while at it.
> + /* Note, previously __UPPER_REGS_DF__ was defined if the option
> + -mupper-regs-df was used and it was on by default for -mvsx. That option
> + is now eliminated, so set __UPPER_REGS_DF__ based on whether VSX was
> + set. */
"That option" reads as refering to -mvsx.
> --- gcc/config/rs6000/rs6000.c (.../svn+ssh://meissner@gcc.gnu.org/svn/gcc/trunk/gcc/config/rs6000) (revision 250405)
> +++ gcc/config/rs6000/rs6000.c (.../gcc/config/rs6000) (working copy)
> @@ -2908,8 +2908,7 @@ rs6000_setup_reg_addr_masks (void)
> && !FLOAT128_VECTOR_P (m2)
> && !complex_p
> && !small_int_vsx_p
> - && (m2 != DFmode || !TARGET_UPPER_REGS_DF)
> - && (m2 != SFmode || !TARGET_UPPER_REGS_SF))
> + && (m2 != DFmode || !TARGET_UPPER_REGS_DF))
> {
> addr_mask |= RELOAD_REG_PRE_INCDEC;
>
Why are you leaving DF here? (A later patch will take care of it, but
is it more than an oversight?)
> +/* { dg-final { scan-assembler-times {\mfctidz\M|\mxscvdpsxds\M} 2 } } */
You could write {\m(fctidz|xscvdpsxds)\M} which may be easier to read.
Come to think of it, we could do e.g. {\m(lwz)\M} as well. Not sure
which is nicer.
Looks good, please commit! Thanks again,
Segher