Re: [RFA] PowerPC e5500 and e6500 cores support

[Michael Meissner <meissner at linux dot vnet dot ibm dot com>]

On Tue, Mar 06, 2012 at 11:44:08AM -0500, Edmar wrote:
> Freescale would like to contribute these patches to gcc.
> 
> It enables gcc for the new Freescale 64 bit cores. It creates a pipeline
> description,  and set proper default flags for the e5500 and e6500 cores.
> 
> Both are 64 bit cores capable to execute popcntb/w/d, bperm, cmpb,
> and prtyw/d instructions.
> 
> The e6500 core has Altivec and also the new Altivec instructions that
> will be part of Power ISA-2.07.
> Several tests cases for the new altivec builtins are included.
> 
> The patch was generated from subversion revision 184757.
> 
> The patch was regression tested for power7 target under these conditions:
> --enable-checking --disable-decimal-float --enable-languages=c,c++,fortran
> 
> During the development process, an ICE for cell target was found.
> The e6500 patch also fixes that problem.
> 
> Since the cell ICE is an regression, I have a separate patch and
> ChangeLog that can be applied against gcc-4.7/4.6/4.5 to fix this ICE only.
> (The branches were also regression tested using the same conditions above)
> 
> Regarding the implementation of popcntb, popcntd, and cmpb. Gcc has
> dedicated masks on target_flags for them, but due to shortage of bits,
> those masks controls more than the name implies.
> 
> TARGET_POPCNTB also controls FP reciprocal estimate that the
> Frescale cores does not have
>
> TARGET_POPCNTD also controls FP double word conversion, lfiwzx that
> the Freescale cores does not have.
>
> TARGET_CMPB also controls copy sign, lfiwax that the Freescale cores
> does not have.

As currently used in the compiler:

  * TARGET_POPCNTB is for all insns in ISA 2.04,
  * TARGET_CMPB is for all insns in ISA 2.05 other than Altivec, Decimal
  * TARGET_POPCNTD is for all insns in ISA 2.06 other than Altivec, Decimal,
    and VSX

> In the patch I minimized the number of changes, while not adding
> any new mask to target_flags.

While we may get some bits back when the original Power flags are removed, it
may be time to bite the bullet and have two separate target flags like x86 did,
because we keep running out of bits.

> A new attribute type "popcnt" is created. This is used in our scheduler,
> since it takes 2 cycles on Freescale cores. The scheduler of current
> architectures are not affected, because the default value of popcnt type
> is the same as not having a type definition on a define_insn.

It is 2 cycles on power6/power7 so this is reasonable.

> We thanks in advance for your time to review and commit these patches

Some comments from looking at the patches:

A meta-issue is the name of the Freescale extensions.  The problem of calling
it Altivec2 is we get into a situation like AMD and Intel have had over what
the next SSE revision is.  Perhaps using a different name than Altivec2.  David
probably needs to weigh in on this.

What is the rationale for changing modes for stv* from V4SI to V16QI, and is it
safe?  I'm just worried about existing code, that suddenly stops working.

In rs6000.c, I think gpopt/mfocrf should only be cleared if the user did not
explicitly set those options.  If you want to issue an error if the user
explicitly set the options for the new cpus, that is fine, but I don't like the
backend to silently change options that were explicitly set on the command
line.

In terms of the comments about the insns being in ISA 2.07, that may be
premature until the ISA is actually published.  Also, some of the Altivec2
insns are not in the ISA 2.07 versions I've reviewed (the trouble is finding
which insns are in which RFCs).

I really don't like the explict CPU checks in TARGET_LFIWAX, TARGET_LFIWZX,
because it is easy to miss when the next new cpu comes out.  Perhaps it would
be better to have one more target flag that says to ignore the instructions
Freescale doesn't support.  I dunno, or as I said, maybe it is time to have two
target_flags.  The x86 has also gone to using HOST_WIDE_INT for their two sets
of target flags.

As I mentioned earlier, I like the new popcnt type attribute, but whenever you
add a new type attribute, you should to go through all of the *.md files add
add support for the new attribute.  Now, insns that aren't supported on older
machines are one thing, but since popcnt has been part of the architecture
since the Power/PowerPC came out, I would think you need to edit power4.md,
7xx.md, etc. to add popcnt in the same place as integer.

I think you need to refine the tests, and change powerpc_altivec_ok lines to
something like:
/* { dg-require-effective-target powerpc_altivec2_ok } */

And then add in support in testsuite/lib/target-supports.exp to detect whether
the assembler supports all of the Altivec2 (or whatever we call it)
instructions.  In the same vein, you probably need to add support in
configure.ac to detect whether the assembler knows about the insns similar to
like I did in gcc_cv_as_powerpc_vsx to detect if the assembler knows about
VSX.

-- 
Michael Meissner, IBM
5 Technology Place Drive, M/S 2757, Westford, MA 01886-3141, USA
meissner@linux.vnet.ibm.com	fax +1 (978) 399-6899