Hi Mike,

On Fri, Nov 02, 2018 at 02:37:34PM -0400, Michael Meissner wrote:
> This patch removes all of the so-called power9 fusion support for the GCC
> compiler.  It leaves -mpower9-fusion as a deprecated switch in case somebody
> used it (the switch was never documented).

As Mike Stump says, please just remove it.  The option was never documented,
most likely zero people use it, and those that do shouldn't have and can
easily adjust.

> [gcc]
> 2018-11-02  Michael Meissner  <meiss...@linux.ibm.com>
> 
>       * config/rs6000/constraints.md (wF constraint): Only document the
>       wF constraint for power8 fusion.  Remove documentation for power9
>       fusion.

It wasn't documented as being anything for p8 before.  So that was wrong?

>       * config/rs6000/predicates.md (p9_fusion_reg_operand): Delete, the
>       predicate only used for power9 fusion support.

Just "Delete."

>       (fusion_gpr_addis): Drop support to allow fusing offsets where the
>       top 11 bits weren't all 0 or all 1's.
>       (fusion_gpr_mem_load): Add comment about not allowing SFmode or
>       DFmode in power8 fusion.
>       (fusion_addis_mem_combo_load): Drop power9 fusion support.  Only
>       support power8 fusion.  Add comment about not allowing SFmode or
>       DFmode in power8 fusion.
>       (fusion_offsettable_mem_operand): Delete, the predicate only used
>       for power9 fusion support.

Just "Delete."  The changelog should not explain *why* you are changing
something.  Put that in the commit message.  The changelog is much easier
to read without this.

>       * config/rs6000/rs6000-cpus.def (ISA_2_7_MASKS_NO_FUSION): New
>       option masks that is all of the ISA 2.07 bits except for fusion.
>       (ISA_2_7_MASKS_SERVER): Add fusion bits back in.
>       (ISA_3_0_MASKS_SERVER): Delete power9 fusion.  Use ISA 2.07 bits
>       without enabling power8 fusion.
>       (POWERPC_MASKS): Delete power9 fusion option mask.

>       * config/rs6000/rs6000-protos.h (emit_fusion_load_store): Delete
>       function declarations used for power9 fusion.
>       (fusion_p9_p): Likewise.
>       (expand_fusion_p9_load): Likewise.
>       (expand_fusion_p9_store): Likewise.
>       (emit_fusion_p9_load): Likewise.
>       (emit_fusion_p9_store): Likewise.

All of these are just "Delete."  (Or "Delete declaration.")

>       * config/rs6000/rs6000.c (rs6000_debug_reg_global): Delete power9
>       fusion debug print.

"Delete."  Well you get the idea...  There are more of them later in here.

>       (rs6000_option_override_internal): Delete power9 fusion option
>       support.  If we do -mcpu=power8 -mtune=power9, turn off power8
>       fusion.

That doesn't sound right.  Either the -mcpu= or the -mtune= should turn
it on, but neither should turn it off.  It sounds like you want -mtune
to say whether fusion is enabled or not?  That sounds fine, but this
should be implemented more directly (or more generically).

>       (rs6000_opt_masks): Delete power9 fusion option.
>       (emit_fusion_load): Rename function from emit_fusion_load_store.
>       Make the function static.  Delete support for fusing stores, since
>       we are deleting power9 fusion.
>       (fusion_p9_p): Delete power9 fusion support functions.
>       (expand_fusion_p9_load): Likewise.
>       (expand_fusion_p9_store): Likewise.
>       (emit_fusion_p9_load): Likewise.
>       (emit_fusion_p9_store): Likewise.
>       * config/rs6000/rs6000.h: Delete comment about power9 fusion.
>       * config/rs6000/rs6000.md (UNSPEC_FUSION_P9): Delete, no longer
>       used since power9 fusion support has been deleted.
>       (GPR_FUSION iterator): Likewise.
>       (FPR_FUSION iterator): Likewise.
>       (power9 fusion peephole2's): Likewise.
>       (fusion_gpr_<P:mode>_<GPR_FUSION:mode>_load): Likewise.
>       (fusion_gpr_<P:mode>_<GPR_FUSION:mode>_store): Likewise.
>       (fusion_vsx_<P:mode>_<FPR_FUSION:mode>_load): Likewise.
>       (fusion_vsx_<P:mode>_<FPR_FUSION:mode>_store): Likewise.
>       (fusion_p9_<mode>_constant): Likewise.
>       * config/rs6000/rs6000.opt (-mpower9-fusion): Mark as deprecated.
>       * doc/md.texi (PowerPC constraints): Update wF documentation.
> 
> [gcc/testsuite]
> 2018-11-02  Michael Meissner  <meiss...@linux.ibm.com>
> 
>       * gcc.target/powerpc/fusion3.c: Delete power9 fusion.
>       * gcc.target/powerpc/fusion4.c: Likewise.


> --- gcc/config/rs6000/rs6000.opt      
> (.../svn+ssh://meiss...@gcc.gnu.org/svn/gcc/trunk/gcc/config/rs6000)    
> (revision 265537)
> +++ gcc/config/rs6000/rs6000.opt      (.../gcc/config/rs6000) (working copy)
> @@ -499,8 +499,7 @@ Target Undocumented Var(rs6000_optimize_
>  Analyze and remove doubleword swaps from VSX computations.
>  
>  mpower9-fusion
> -Target Undocumented Report Mask(P9_FUSION) Var(rs6000_isa_flags)
> -Fuse certain operations together for better performance on power9.
> +Target Undocumented Mask(P9_FUSION) Var(rs6000_isa_flags) Deprecated

Yeah just delete this please.

> @@ -1692,11 +1650,7 @@ (define_predicate "fusion_gpr_addis"
>      return 0;
>  
>    /* Power8 currently will only do the fusion if the top 11 bits of the addis
> -     value are all 1's or 0's.  Ignore this restriction if we are testing
> -     advanced fusion.  */
> -  if (TARGET_P9_FUSION)
> -    return 1;
> -
> +     value are all 1's or 0's.  */
>    return (IN_RANGE (value >> 16, -32, 31));
>  })

I think this is top 12 bits equal, not 11, so [-16..15].

> @@ -1762,14 +1718,13 @@ (define_predicate "fusion_gpr_mem_load"
>  ;; Match a GPR load (lbz, lhz, lwz, ld) that uses a combined address in the
>  ;; memory field with both the addis and the memory offset.  Sign extension
>  ;; is not handled here, since lha and lwa are not fused.
> -;; With P9 fusion, also match a fpr/vector load and float_extend
>  (define_predicate "fusion_addis_mem_combo_load"
>    (match_code "mem,zero_extend,float_extend")

So float_extend should be deleted here?


Segher

Reply via email to