Re: [PATCH], V7, #3 of 7, Use PADDI for 34-bit immediate adds

2019-11-25 Thread Segher Boessenkool 
On Mon, Nov 25, 2019 at 05:40:02PM -0500, Michael Meissner wrote:
> On Fri, Nov 22, 2019 at 06:32:08PM -0600, Segher Boessenkool wrote:
> > On Thu, Nov 14, 2019 at 05:44:42PM -0500, Michael Meissner wrote:
> > > This patch generates PADDI to add 34-bit immediate constants on the 
> > > 'future'
> > > system, and prevents such adds from being split.
> > 
> > I don't see that last part?  Is that a remnant of a previous version of
> > the patch?
> 
> This falls out of the add_operand change.



> Since non_add_cint_operand is defined as:
> 
> (define_predicate "non_add_cint_operand"
>   (and (match_code "const_int")
>(not (match_operand 0 "add_operand"
> 
> It follows that if add_operand now allows a 34-bit constant when -mcpu=future,
> that non_add_cint_operand will return 0, and the add will not be split.

Right, so write that please ("changes add_operand"), not some random
consequence of that change.

> > >   * config/rs6000/rs6000.md (add3): Add support for PADDI.
> > 
> > (add3 for GPR): etc.
> > 
> > We have six things all called add3 (VI2, DDTD, SDI, SFDF, IEEE128,
> > VEC_F).  Erm.
> > 
> > Wait, this is called *add3, not add3.
> 
> Yeah, I usually add the iterator part to identify which one was changed.

No, the asterisk is part of the name, that's my point.  And yes please
say what the iterator is.


Segher

Re: [PATCH], V7, #3 of 7, Use PADDI for 34-bit immediate adds

2019-11-25 Thread Michael Meissner 
On Fri, Nov 22, 2019 at 06:32:08PM -0600, Segher Boessenkool wrote:
> On Thu, Nov 14, 2019 at 05:44:42PM -0500, Michael Meissner wrote:
> > This patch generates PADDI to add 34-bit immediate constants on the 'future'
> > system, and prevents such adds from being split.
> 
> I don't see that last part?  Is that a remnant of a previous version of
> the patch?

This falls out of the add_operand change.

The add splitter is:

;; Split an add that we can't do in one insn into two insns, each of which
;; does one 16-bit part.  This is used by combine.  Note that the low-order
;; add should be last in case the result gets used in an address.

(define_split
  [(set (match_operand:GPR 0 "gpc_reg_operand")
(plus:GPR (match_operand:GPR 1 "gpc_reg_operand")
  (match_operand:GPR 2 "non_add_cint_operand")))]
  ""
  [(set (match_dup 0) (plus:GPR (match_dup 1) (match_dup 3)))
   (set (match_dup 0) (plus:GPR (match_dup 0) (match_dup 4)))]
{
  HOST_WIDE_INT val = INTVAL (operands[2]);
  HOST_WIDE_INT low = ((val & 0x) ^ 0x8000) - 0x8000;
  HOST_WIDE_INT rest = trunc_int_for_mode (val - low, mode);

  operands[4] = GEN_INT (low);
  if (mode == SImode || satisfies_constraint_L (GEN_INT (rest)))
operands[3] = GEN_INT (rest);
  else if (can_create_pseudo_p ())
{
  operands[3] = gen_reg_rtx (DImode);
  emit_move_insn (operands[3], operands[2]);
  emit_insn (gen_adddi3 (operands[0], operands[1], operands[3]));
  DONE;
}
  else
FAIL;
})

Since non_add_cint_operand is defined as:

(define_predicate "non_add_cint_operand"
  (and (match_code "const_int")
   (not (match_operand 0 "add_operand"

It follows that if add_operand now allows a 34-bit constant when -mcpu=future,
that non_add_cint_operand will return 0, and the add will not be split.

> > * config/rs6000/predicates.md (add_operand): Add support for
> > PADDI.
> 
> More directly: Allow constants that satisfy eI.

Ok.

> > * config/rs6000/rs6000.md (add3): Add support for PADDI.
> 
> (add3 for GPR): etc.
> 
> We have six things all called add3 (VI2, DDTD, SDI, SFDF, IEEE128,
> VEC_F).  Erm.
> 
> Wait, this is called *add3, not add3.

Yeah, I usually add the iterator part to identify which one was changed.
> 
> Okay for trunk with that fixed.  Thanks,
> 
> 
> Segher

-- 
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

Re: [PATCH], V7, #3 of 7, Use PADDI for 34-bit immediate adds

2019-11-22 Thread Segher Boessenkool 
On Thu, Nov 14, 2019 at 05:44:42PM -0500, Michael Meissner wrote:
> This patch generates PADDI to add 34-bit immediate constants on the 'future'
> system, and prevents such adds from being split.

I don't see that last part?  Is that a remnant of a previous version of
the patch?

>   * config/rs6000/predicates.md (add_operand): Add support for
>   PADDI.

More directly: Allow constants that satisfy eI.

>   * config/rs6000/rs6000.md (add3): Add support for PADDI.

(add3 for GPR): etc.

We have six things all called add3 (VI2, DDTD, SDI, SFDF, IEEE128,
VEC_F).  Erm.

Wait, this is called *add3, not add3.


Okay for trunk with that fixed.  Thanks,


Segher