This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH], Add power9 support to GCC, patch #2 (add modulus instructions)
- From: Michael Meissner <meissner at linux dot vnet dot ibm dot com>
- To: Segher Boessenkool <segher at kernel dot crashing dot org>
- Cc: Michael Meissner <meissner at linux dot vnet dot ibm dot com>, gcc-patches at gcc dot gnu dot org, dje dot gcc at gmail dot com
- Date: Mon, 9 Nov 2015 13:07:39 -0500
- Subject: Re: [PATCH], Add power9 support to GCC, patch #2 (add modulus instructions)
- Authentication-results: sourceware.org; auth=none
- References: <20151103202911 dot GA5304 at ibm-tiger dot the-meissners dot org> <20151109003616 dot GB17170 at ibm-tiger dot the-meissners dot org> <20151109154850 dot GA6528 at gate dot crashing dot org>
On Mon, Nov 09, 2015 at 09:48:50AM -0600, Segher Boessenkool wrote:
> Hi,
>
> On Sun, Nov 08, 2015 at 07:36:16PM -0500, Michael Meissner wrote:
> > [gcc/testsuite]
> > * lib/target-supports.exp (check_p9vector_hw_available): Add
> > checks for power9 availability.
> > (check_effective_target_powerpc_p9vector_ok): Likewise.
>
> It's probably better not to use this for modulo; it is confusing and if
> you'll later need to untangle it it is much more work.
>
> > +/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
> > +/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
>
> Lose this line? If Darwin cannot support modulo, the next line will
> catch that.
>
> +/* { dg-require-effective-target powerpc_p9vector_ok } */
> > +/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power9" } } */
> > +/* { dg-options "-mcpu=power9 -O3" } */
>
> Is -O3 needed? Why won't -O2 work?
Just habit.
> > +proc check_p9vector_hw_available { } {
> > + return [check_cached_effective_target p9vector_hw_available {
> > + # Some simulators are known to not support VSX/power8 instructions.
> > + # For now, disable on Darwin
> > + if { [istarget powerpc-*-eabi] || [istarget powerpc*-*-eabispe] || [istarget *-*-darwin*]} {
>
> Long line.
Cut and paste from other tests.
> > Index: gcc/config/rs6000/rs6000.md
> > ===================================================================
> > --- gcc/config/rs6000/rs6000.md (revision 229972)
> > +++ gcc/config/rs6000/rs6000.md (working copy)
> > @@ -2885,9 +2885,9 @@ (define_insn_and_split "*div<mode>3_sra_
> > (set_attr "cell_micro" "not")])
> >
> > (define_expand "mod<mode>3"
> > - [(use (match_operand:GPR 0 "gpc_reg_operand" ""))
> > - (use (match_operand:GPR 1 "gpc_reg_operand" ""))
> > - (use (match_operand:GPR 2 "reg_or_cint_operand" ""))]
> > + [(set (match_operand:GPR 0 "gpc_reg_operand" "")
> > + (mod:GPR (match_operand:GPR 1 "gpc_reg_operand" "")
> > + (match_operand:GPR 2 "reg_or_cint_operand" "")))]
>
> You could delete the empty constraint strings while you're at it.
>
> > +;; On machines with modulo support, do a combined div/mod the old fashioned
> > +;; method, since the multiply/subtract is faster than doing the mod instruction
> > +;; after a divide.
>
> You can instead have a "divmod" insn that is split to either of div, mod,
> or div+mul+sub depending on which of the outputs is unused. Peepholes
> do not get all cases.
Yes, though as I recall, I couldn't get it to do what I wanted, and moved on to
other targets.
> This can be a later improvement of course.
Yep.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797