On Mon, Nov 05, 2018 at 04:09:23PM -0600, Segher Boessenkool wrote:
> 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?
The switch wasn't documented. In the constraint (which is what I'm changing
here), the constraint mentioned p9 fusion in the documentation string.
> > (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).
Ok, I will look at it.
> > 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.
Ok.
> > @@ -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].
It is 11 bits, check section 12.1.12 in the power8 book IV.
addis(SI) first 11 bits must be all 0’s or all 1’s
> > @@ -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?
Yes.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.ibm.com, phone: +1 (978) 899-4797
