Re: [PATCH, ARM/AArch64] drop aarch32 support for falkor/qdf24xx

[Jim Wilson]

On Mon, Jun 12, 2017 at 3:40 AM, James Greenhalgh
 wrote:
> In both the original patch, and the backport, you're modifying the
> AArch64 options here. I'd expect the edits to be to the AArch32 options
> (these start somewhere around line 15,000).

Yes, I screwed this up.  Richard Earnshaw already fixed the ARM Option
list in one of his -mcpu patches.  I checked in a fix for the AArch64
Option list under the obvious rule.  Tested with a make doc, and using
info to look at the docs to make sure that they are right.

I will fix the gcc-7 backport before I check it in.

Jim
	gcc/
	* doc/invoke.texi (AArch64 Options, -mtune): Re-add falkor and
	qdf24xx.

Index: doc/invoke.texi
===
--- doc/invoke.texi	(revision 249611)
+++ doc/invoke.texi	(working copy)
@@ -14079,7 +14079,8 @@ Specify the name of the target processor for which
 performance of the code.  Permissible values for this option are:
 @samp{generic}, @samp{cortex-a35}, @samp{cortex-a53}, @samp{cortex-a55},
 @samp{cortex-a57}, @samp{cortex-a72}, @samp{cortex-a73}, @samp{cortex-a75},
-@samp{exynos-m1}, @samp{xgene1}, @samp{vulcan}, @samp{thunderx},
+@samp{exynos-m1}, @samp{falkor}, @samp{qdf24xx},
+@samp{xgene1}, @samp{vulcan}, @samp{thunderx},
 @samp{thunderxt88}, @samp{thunderxt88p1}, @samp{thunderxt81},
 @samp{thunderxt83}, @samp{thunderx2t99}, @samp{cortex-a57.cortex-a53},
 @samp{cortex-a72.cortex-a53}, @samp{cortex-a73.cortex-a35},

Re: [PATCH, ARM/AArch64] drop aarch32 support for falkor/qdf24xx

[James Greenhalgh]

On Fri, Jun 09, 2017 at 01:03:34PM -0700, Jim Wilson wrote:
>  # Arch Matches
> Index: gcc/doc/invoke.texi
> ===
> --- gcc/doc/invoke.texi   (revision 249025)
> +++ gcc/doc/invoke.texi   (working copy)
> @@ -13983,8 +13983,8 @@ processors implementing the target architecture.
>  Specify the name of the target processor for which GCC should tune the
>  performance of the code.  Permissible values for this option are:
>  @samp{generic}, @samp{cortex-a35}, @samp{cortex-a53}, @samp{cortex-a57},
> -@samp{cortex-a72}, @samp{cortex-a73}, @samp{exynos-m1}, @samp{falkor},
> -@samp{qdf24xx}, @samp{xgene1}, @samp{vulcan}, @samp{thunderx},
> +@samp{cortex-a72}, @samp{cortex-a73}, @samp{exynos-m1},
> +@samp{xgene1}, @samp{vulcan}, @samp{thunderx},
>  @samp{thunderxt88}, @samp{thunderxt88p1}, @samp{thunderxt81},
>  @samp{thunderxt83}, @samp{thunderx2t99}, @samp{cortex-a57.cortex-a53},
>  @samp{cortex-a72.cortex-a53}, @samp{cortex-a73.cortex-a35},

In both the original patch, and the backport, you're modifying the
AArch64 options here. I'd expect the edits to be to the AArch32 options
(these start somewhere around line 15,000).

Thanks,
James

Re: [PATCH, ARM/AArch64] drop aarch32 support for falkor/qdf24xx

[Richard Earnshaw (lists)]

On 09/06/17 21:03, Jim Wilson wrote:
> On Thu, May 25, 2017 at 2:25 AM, Richard Earnshaw (lists)
>  wrote:
>> Having pondered this over night, I think the lowest risk thing to do,
>> provided it applies cleanly to the gcc-7 branch, is just commit the
>> entire patch on the branch and be done with it.  The risk from removing
>> this code is pretty minimal and removing it all is the best way of
>> avoiding things like unexpected compiler warnings breaking the build.
>> If it doesn't apply cleanly, then just drop the documentation.
> 
> The patch backports cleanly.  I've tested the patch on the
> gcc-7-branch, with both armv8 and aarch64 bootstraps and make checks,
> with no regressions.  Ok to check in?
> 
> Jim
> 

OK.

R.

Re: [PATCH, ARM/AArch64] drop aarch32 support for falkor/qdf24xx

[Jim Wilson]

On Thu, May 25, 2017 at 2:25 AM, Richard Earnshaw (lists)
 wrote:
> Having pondered this over night, I think the lowest risk thing to do,
> provided it applies cleanly to the gcc-7 branch, is just commit the
> entire patch on the branch and be done with it.  The risk from removing
> this code is pretty minimal and removing it all is the best way of
> avoiding things like unexpected compiler warnings breaking the build.
> If it doesn't apply cleanly, then just drop the documentation.

The patch backports cleanly.  I've tested the patch on the
gcc-7-branch, with both armv8 and aarch64 bootstraps and make checks,
with no regressions.  Ok to check in?

Jim
	gcc/
	* config/aarch64/aarch64-cost-tables.h (qdf24xx_extra_costs): Move to
	here.
	* config/arm/aarch-cost-tables.h (qdf24xx_extra_costs): From here.
	* config/arm/arm-cpu-cdata.h: Regenerate.
	* config/arm/arm-cpu-data.h, config/arm/arm-cpu.h: Likewise.
	* config/arm/arm-tables.opt, config/arm/arm-tune.md: Likewise.
	* config/arm/arm-cpus.in: Delete falkor and qdf24xx entries.
	* config/arm/arm.c (arm_qdf24xx_tune): Delete.
	* config/arm/bpabi.h (BE8_LINK_SPEC): Delete falkor and qdf24xx
	support.
	* config/arm/t-aprofile (MULTILIB_MATCHES): Delete falkor and qdf24xx
	support.
	* config/arm/t-rmprofile: Likewise.
	* doc/invoke.texi (ARM Options): Drop falkor and qdf24xx support.

Index: gcc/config/aarch64/aarch64-cost-tables.h
===
--- gcc/config/aarch64/aarch64-cost-tables.h	(revision 249025)
+++ gcc/config/aarch64/aarch64-cost-tables.h	(working copy)
@@ -23,7 +23,111 @@
 
 #include "config/arm/aarch-cost-tables.h"
 
-/* ThunderX does not have implement AArch32.  */
+/* QDF24xx does not implement AArch32.  */
+const struct cpu_cost_table qdf24xx_extra_costs =
+{
+  /* ALU */
+  {
+0, /* arith.  */
+0, /* logical.  */
+0, /* shift.  */
+0, /* shift_reg.  */
+COSTS_N_INSNS (1), /* arith_shift.  */
+COSTS_N_INSNS (1), /* arith_shift_reg.  */
+0, /* log_shift.  */
+0, /* log_shift_reg.  */
+0, /* extend.  */
+0, /* extend_arith.  */
+0, /* bfi.  */
+0, /* bfx.  */
+0, /* clz.  */
+0,	   /* rev.  */
+0, /* non_exec.  */
+true   /* non_exec_costs_exec.  */
+  },
+  {
+/* MULT SImode */
+{
+  COSTS_N_INSNS (2),   /* simple.  */
+  COSTS_N_INSNS (2),   /* flag_setting.  */
+  COSTS_N_INSNS (2),   /* extend.  */
+  COSTS_N_INSNS (2),   /* add.  */
+  COSTS_N_INSNS (2),   /* extend_add.  */
+  COSTS_N_INSNS (4)   /* idiv.  */
+},
+/* MULT DImode */
+{
+  COSTS_N_INSNS (3),   /* simple.  */
+  0,   /* flag_setting (N/A).  */
+  COSTS_N_INSNS (3),   /* extend.  */
+  COSTS_N_INSNS (3),   /* add.  */
+  COSTS_N_INSNS (3),   /* extend_add.  */
+  COSTS_N_INSNS (9)   /* idiv.  */
+}
+  },
+  /* LD/ST */
+  {
+COSTS_N_INSNS (2), /* load.  */
+COSTS_N_INSNS (2), /* load_sign_extend.  */
+COSTS_N_INSNS (2), /* ldrd.  */
+COSTS_N_INSNS (2), /* ldm_1st.  */
+1, /* ldm_regs_per_insn_1st.  */
+2, /* ldm_regs_per_insn_subsequent.  */
+COSTS_N_INSNS (2), /* loadf.  */
+COSTS_N_INSNS (2), /* loadd.  */
+COSTS_N_INSNS (3), /* load_unaligned.  */
+0, /* store.  */
+0, /* strd.  */
+0, /* stm_1st.  */
+1, /* stm_regs_per_insn_1st.  */
+2, /* stm_regs_per_insn_subsequent.  */
+0, /* storef.  */
+0, /* stored.  */
+COSTS_N_INSNS (1), /* store_unaligned.  */
+COSTS_N_INSNS (1), /* loadv.  */
+COSTS_N_INSNS (1)  /* storev.  */
+  },
+  {
+/* FP SFmode */
+{
+  COSTS_N_INSNS (6),  /* div.  */
+  COSTS_N_INSNS (5),   /* mult.  */
+  COSTS_N_INSNS (5),   /* mult_addsub. */
+  COSTS_N_INSNS (5),   /* fma.  */
+  COSTS_N_INSNS (3),   /* addsub.  */
+  COSTS_N_INSNS (1),   /* fpconst. */
+  COSTS_N_INSNS (1),   /* neg.  */
+  COSTS_N_INSNS (2),   /* compare.  */
+  COSTS_N_INSNS (4),   /* widen.  */
+  COSTS_N_INSNS (4),   /* narrow.  */
+  COSTS_N_INSNS (4),   /* toint.  */
+  COSTS_N_INSNS (4),   /* fromint.  */
+  COSTS_N_INSNS (2)/* roundint.  */
+},
+/* FP DFmode */
+{
+  COSTS_N_INSNS (11),  /* div.  */
+  COSTS_N_INSNS (6),   /* mult.  */
+  COSTS_N_INSNS (6),   /* 

Re: [PATCH, ARM/AArch64] drop aarch32 support for falkor/qdf24xx

[Richard Earnshaw (lists)]

On 24/05/17 17:19, Richard Earnshaw (lists) wrote:
> On 24/05/17 17:03, Jim Wilson wrote:
>> On Wed, May 24, 2017 at 8:17 AM, Richard Earnshaw (lists)
>>  wrote:
>>> On 24/05/17 15:18, Jim Wilson wrote:
 On Wed, May 24, 2017 at 6:56 AM, Richard Earnshaw (lists)
  wrote:
> OK.  does this need to go in the gcc-8 changes file?

 Falkor hasn't shipped yet.  I'm dropping features that only existed in
 preproduction NDA hardware, so there isn't anything end user visible,
 and hence I don't think that it needs to be in the release notes.

 Jim

>>>
>>> Fair enough, so what about a minimal back-port to GCC-7 that just
>>> disables the CPU name for aarch32?
>>
>> Not sure how to do that.  If I remove the arm-cpus.in entry, then 5
>> files get automatically regenerated.  That leaves us with a few minor
>> inconsistencies in specs handling and multilibs which are harmless but
>> we may as well fix anyways.  The only part of the patch that is
>> optional if the part which moves the qdf24xx_extra_costs array from
>> the arm dir to the aarch64 dir.  So the minimal patch ends up being
>> half the size of the original patch, changing 9 of the original 11
>> files, which isn't very minimal.
>>
>> Another option might be to just remove the documentation and leave the
>> code in, i.e. only apply the doc/invoke.texi patch.  That would be a
>> small and safe patch.
>>
>> Jim
>>
> 
> Certainly we should remove it from the documentation.  That might be the
> best idea.
> 
> I don't really regard the size of the changes to the auto-generated code
> as being relevant - if we put the generated code directly in the build
> directory and treated it like we do the output from gen*.c, then those
> changes would never be even noticed.
> 
> R.
> 

Having pondered this over night, I think the lowest risk thing to do,
provided it applies cleanly to the gcc-7 branch, is just commit the
entire patch on the branch and be done with it.  The risk from removing
this code is pretty minimal and removing it all is the best way of
avoiding things like unexpected compiler warnings breaking the build.
If it doesn't apply cleanly, then just drop the documentation.

R.

Re: [PATCH, ARM/AArch64] drop aarch32 support for falkor/qdf24xx

[Richard Earnshaw (lists)]

On 24/05/17 17:03, Jim Wilson wrote:
> On Wed, May 24, 2017 at 8:17 AM, Richard Earnshaw (lists)
>  wrote:
>> On 24/05/17 15:18, Jim Wilson wrote:
>>> On Wed, May 24, 2017 at 6:56 AM, Richard Earnshaw (lists)
>>>  wrote:
 OK.  does this need to go in the gcc-8 changes file?
>>>
>>> Falkor hasn't shipped yet.  I'm dropping features that only existed in
>>> preproduction NDA hardware, so there isn't anything end user visible,
>>> and hence I don't think that it needs to be in the release notes.
>>>
>>> Jim
>>>
>>
>> Fair enough, so what about a minimal back-port to GCC-7 that just
>> disables the CPU name for aarch32?
> 
> Not sure how to do that.  If I remove the arm-cpus.in entry, then 5
> files get automatically regenerated.  That leaves us with a few minor
> inconsistencies in specs handling and multilibs which are harmless but
> we may as well fix anyways.  The only part of the patch that is
> optional if the part which moves the qdf24xx_extra_costs array from
> the arm dir to the aarch64 dir.  So the minimal patch ends up being
> half the size of the original patch, changing 9 of the original 11
> files, which isn't very minimal.
> 
> Another option might be to just remove the documentation and leave the
> code in, i.e. only apply the doc/invoke.texi patch.  That would be a
> small and safe patch.
> 
> Jim
> 

Certainly we should remove it from the documentation.  That might be the
best idea.

I don't really regard the size of the changes to the auto-generated code
as being relevant - if we put the generated code directly in the build
directory and treated it like we do the output from gen*.c, then those
changes would never be even noticed.

R.

Re: [PATCH, ARM/AArch64] drop aarch32 support for falkor/qdf24xx

[Jim Wilson]

On Wed, May 24, 2017 at 8:17 AM, Richard Earnshaw (lists)
 wrote:
> On 24/05/17 15:18, Jim Wilson wrote:
>> On Wed, May 24, 2017 at 6:56 AM, Richard Earnshaw (lists)
>>  wrote:
>>> OK.  does this need to go in the gcc-8 changes file?
>>
>> Falkor hasn't shipped yet.  I'm dropping features that only existed in
>> preproduction NDA hardware, so there isn't anything end user visible,
>> and hence I don't think that it needs to be in the release notes.
>>
>> Jim
>>
>
> Fair enough, so what about a minimal back-port to GCC-7 that just
> disables the CPU name for aarch32?

Not sure how to do that.  If I remove the arm-cpus.in entry, then 5
files get automatically regenerated.  That leaves us with a few minor
inconsistencies in specs handling and multilibs which are harmless but
we may as well fix anyways.  The only part of the patch that is
optional if the part which moves the qdf24xx_extra_costs array from
the arm dir to the aarch64 dir.  So the minimal patch ends up being
half the size of the original patch, changing 9 of the original 11
files, which isn't very minimal.

Another option might be to just remove the documentation and leave the
code in, i.e. only apply the doc/invoke.texi patch.  That would be a
small and safe patch.

Jim

Re: [PATCH, ARM/AArch64] drop aarch32 support for falkor/qdf24xx

[Richard Earnshaw (lists)]

On 24/05/17 15:18, Jim Wilson wrote:
> On Wed, May 24, 2017 at 6:56 AM, Richard Earnshaw (lists)
>  wrote:
>> OK.  does this need to go in the gcc-8 changes file?
> 
> Falkor hasn't shipped yet.  I'm dropping features that only existed in
> preproduction NDA hardware, so there isn't anything end user visible,
> and hence I don't think that it needs to be in the release notes.
> 
> Jim
> 

Fair enough, so what about a minimal back-port to GCC-7 that just
disables the CPU name for aarch32?

R.

Re: [PATCH, ARM/AArch64] drop aarch32 support for falkor/qdf24xx

[Jim Wilson]

On Wed, May 24, 2017 at 6:56 AM, Richard Earnshaw (lists)
 wrote:
> OK.  does this need to go in the gcc-8 changes file?

Falkor hasn't shipped yet.  I'm dropping features that only existed in
preproduction NDA hardware, so there isn't anything end user visible,
and hence I don't think that it needs to be in the release notes.

Jim

Re: [PATCH, ARM/AArch64] drop aarch32 support for falkor/qdf24xx

[Richard Earnshaw (lists)]

On 12/05/17 03:43, Jim Wilson wrote:
> Early steppings had aarch32 support, current steppings don't, so the
> aarch32 support for falkor/qdf24xx needs to be dropped.  This mostly
> involves removing falkor/qdf24xx references from the arm port.  The
> qdf24xx_extra_costs structure moves from the arm port to the aarch64
> port.
> 
> This was tested with aarch64-linux and armv8-linux bootstraps and make
> checks.  There were no regressions.  Also verified by hand that the
> -mcpu=falkor and -mqdf24xx options no longer work in the arm port but
> still work in the aarch64 port.
> 
> Jim
> 

OK.  does this need to go in the gcc-8 changes file?

R.