This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, rs6000] power8 patches, patch #8, power8 load fusion + misc.


On Tue, Jun 18, 2013 at 02:30:49PM -0400, David Edelsohn wrote:
> On Wed, May 22, 2013 at 4:52 PM, Michael Meissner
> <meissner@linux.vnet.ibm.com> wrote:
> 
> > 2013-05-22  Michael Meissner  <meissner@linux.vnet.ibm.com>
> >
> >         * config/rs6000/predicates.md (fusion_gpr_addis): New predicates
> >         to support power8 load fusion.
> >         (fusion_gpr_mem_load): Likewise.
> >
> >         * config/rs6000/rs6000-modes.def (PTImode): Update a comment.
> >
> >         * config/rs6000/rs6000-protos.h (fusion_gpr_load_p): New
> >         declarations for power8 load fusion.
> >         (emit_fusion_gpr_load): Likewise.
> >
> >         * config/rs6000/rs6000.opt (-mlra): New undocumented switch to
> >         turn on using the LRA register allocator.
> >         (-mconstrain-regs): New undocumented switch to constrain
> >         non-integer values from being loaded into the LR or CTR registers.
> 
> This really should have been a separate patch.

Yes, you are right.  I can separate it to be a separate patch if desired.  The
last I checked, there were still problems in moving to use LRA.  It would be
nice if we could get the switch for better testing, rather than continuing to
use a branch.  Right now my focus as been getting the initial power8 changes
in, so it was added more because it was in the sandbox, I was working from.

> +  /* 32-bit is not done yet.  */
> +  if (TARGET_ELF && !TARGET_POWERPC64)
> +    return 0;
> 
> What does "32-bit is not done yet." mean? This means PPC32 Linux is
> not supported but PPC32 AIX is supported?

I don't believe AIX and Linux 64-bit small code model will work with fusion
loading the GPRs, except in the case where you have more than 64K in the static
area that the section anchors point to.  It would work with the VSX fusion that
loads a small constant plus doing hte load.  I tend to feel that restructuring
the code to allow more general addresses before reload, and have secondary
reload, generate the appropriate instructions will work better, but that may
take a longer period to get correct (I'm starting work on it now).

I hadn't gotten around to to looking at 32-bit ELF/Linux.  In theory, 32-bit
Linux should work well with fusion for non-pic code.

> +  if (TARGET_ELF && !TARGET_POWERPC64)
> +    return 0;
> 
> Please return "true" and "false" from new predicates, not "1" and "0".

Ok, I was just being constant with the existing code.

> +
> +    case DImode:
> +      if (TARGET_POWERPC64)
> +    {
> +      mode_name = "long";
> +      load_str = "ld";
> +    }
> +      break;
> 
> What happens for DImode when not TARGET_POWERPC64?  This should be
> gcc_unreachable()?

There is a gcc_unreachable () at the end of the switch that is reached by
either an unknown mode (default case), or by DImode on 32-bit.  But I can put
in two separate gcc_unreachable ()'s.

-- 
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460, USA
email: meissner@linux.vnet.ibm.com, phone: +1 (978) 899-4797


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]