Re: [PATCH] BZ60501: Add addptr optab

2014-03-24 Thread Andreas Krebbel
 I agree with Vlad that we're better off with Andreas' patch than without, 
 since
 computing addresses is going to be 99% of what reload/LRA needs to do.
 
 I also agree with Eric that some better commentary would be nice.

Ok. I've applied the following patch.

Bye,

-Andreas-

2014-03-24  Andreas Krebbel  andreas.kreb...@de.ibm.com

PR rtl-optimization/60501
* optabs.def (addptr3_optab): New optab.
* optabs.c (gen_addptr3_insn, have_addptr3_insn): New function.
* doc/md.texi (addptrm3): Document new RTL standard expander.
* expr.h (gen_addptr3_insn, have_addptr3_insn): Add prototypes.

* lra.c (emit_add3_insn): Use the addptr pattern if available.

* config/s390/s390.md (addptrdi3, addptrsi3): New expanders.

diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index 76902b5..7d9d1ad 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -5034,6 +5034,57 @@
   [(set_attr op_type  RRer,RXE)
(set_attr type fsimpmode)])
 
+;
+; Pointer add instruction patterns
+;
+
+; This will match *la_64
+(define_expand addptrdi3
+  [(set (match_operand:DI 0 register_operand )
+(plus:DI (match_operand:DI 1 register_operand )
+(match_operand:DI 2 nonmemory_operand )))]
+  TARGET_64BIT
+{
+  HOST_WIDE_INT c = INTVAL (operands[2]);
+
+  if (GET_CODE (operands[2]) == CONST_INT)
+{
+  if (!CONST_OK_FOR_CONSTRAINT_P (c, 'K', K)
+  !CONST_OK_FOR_CONSTRAINT_P (c, 'O', Os))
+{
+ operands[2] = force_const_mem (DImode, operands[2]);
+ operands[2] = force_reg (DImode, operands[2]);
+}
+  else if (!DISP_IN_RANGE (INTVAL (operands[2])))
+operands[2] = force_reg (DImode, operands[2]);
+}
+})
+
+; For 31 bit we have to prevent the generated pattern from matching
+; normal ADDs since la only does a 31 bit add.  This is supposed to
+; match force_la_31.
+(define_expand addptrsi3
+  [(parallel
+[(set (match_operand:SI 0 register_operand )
+ (plus:SI (match_operand:SI 1 register_operand )
+  (match_operand:SI 2 nonmemory_operand )))
+  (use (const_int 0))])]
+  !TARGET_64BIT
+{
+  HOST_WIDE_INT c = INTVAL (operands[2]);
+
+  if (GET_CODE (operands[2]) == CONST_INT)
+{
+  if (!CONST_OK_FOR_CONSTRAINT_P (c, 'K', K)
+  !CONST_OK_FOR_CONSTRAINT_P (c, 'O', Os))
+{
+ operands[2] = force_const_mem (SImode, operands[2]);
+ operands[2] = force_reg (SImode, operands[2]);
+}
+  else if (!DISP_IN_RANGE (INTVAL (operands[2])))
+operands[2] = force_reg (SImode, operands[2]);
+}
+})
 
 ;;
 ;;- Subtract instructions.
diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index 746acc2..85fd4b9 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -4720,6 +4720,17 @@ Add operand 2 and operand 1, storing the result in 
operand 0.  All operands
 must have mode @var{m}.  This can be used even on two-address machines, by
 means of constraints requiring operands 1 and 0 to be the same location.
 
+@cindex @code{addptr@var{m}3} instruction pattern
+@item @samp{addptr@var{m}3}
+Like @code{add@var{m}3} but is guaranteed to only be used for address
+calculations.  The expanded code is not allowed to clobber the
+condition code.  It only needs to be defined if @code{add@var{m}3}
+sets the condition code.  If adds used for address calculations and
+normal adds are not compatible it is required to expand a distinct
+pattern (e.g. using an unspec).  The pattern is used by LRA to emit
+address calculations.  @code{add@var{m}3} is used if
+@code{addptr@var{m}3} is not defined.
+
 @cindex @code{ssadd@var{m}3} instruction pattern
 @cindex @code{usadd@var{m}3} instruction pattern
 @cindex @code{sub@var{m}3} instruction pattern
diff --git a/gcc/expr.h b/gcc/expr.h
index 5111f06..524da67 100644
--- a/gcc/expr.h
+++ b/gcc/expr.h
@@ -180,10 +180,12 @@ extern void emit_libcall_block (rtx, rtx, rtx, rtx);
Likewise for subtraction and for just copying.  */
 extern rtx gen_add2_insn (rtx, rtx);
 extern rtx gen_add3_insn (rtx, rtx, rtx);
+extern rtx gen_addptr3_insn (rtx, rtx, rtx);
 extern rtx gen_sub2_insn (rtx, rtx);
 extern rtx gen_sub3_insn (rtx, rtx, rtx);
 extern rtx gen_move_insn (rtx, rtx);
 extern int have_add2_insn (rtx, rtx);
+extern int have_addptr3_insn (rtx, rtx, rtx);
 extern int have_sub2_insn (rtx, rtx);
 
 /* Emit a pair of rtl insns to compare two rtx's and to jump
diff --git a/gcc/lra.c b/gcc/lra.c
index 77074e2..c1b92d8 100644
--- a/gcc/lra.c
+++ b/gcc/lra.c
@@ -254,6 +254,19 @@ emit_add3_insn (rtx x, rtx y, rtx z)
   rtx insn, last;
 
   last = get_last_insn ();
+
+  if (have_addptr3_insn (x, y, z))
+{
+  insn = gen_addptr3_insn (x, y, z);
+
+  /* If the target provides an addptr pattern it hopefully does
+for a reason.  So falling back to the normal add would be
+a bug.  */
+  lra_assert (insn != NULL_RTX);
+  emit_insn (insn);
+  return insn;

Re: [PATCH] BZ60501: Add addptr optab

2014-03-18 Thread Jakub Jelinek
On Mon, Mar 17, 2014 at 03:24:14PM -0400, Vladimir Makarov wrote:
 It is complicated.  There is no guarantee that it is used only for
 addresses.  I need some time to think how to fix it.
 
 Meanwhile, you *should* commit the patch into the trunk because it
 solves the real problem.  And I can work from this to make changes
 that the new pattern is only used for addresses.
 
 The patch is absolutely safe for all targets but s390.  There is
 still a tiny possibility that it might result in some problems for
 s390  (now I see only one situation when a pseudo in a subreg
 changed by equiv plus expr needs a reload).  In any case your patch
 solves real numerous failures and can be used as a base for further
 work.
 
 Thanks for working on this problem, Andreas.  Sorry that I missed
 the PR60501.

BTW, does LRA require that CC isn't clobbered when it uses
emit_add2_insn? I don't see how it can be guaranteed
(except perhaps on i?86/x86_64 and maybe a few other targets).
emit_add3_insn should be ok (even on s390*?) because recog_memoized
should (I think) never add clobbers (it calls recog with 0
as last argument), but gen_add2_insn is a normall addPmode3 insn that
on many targets clobbers CC.

Jakub


Re: [PATCH] BZ60501: Add addptr optab

2014-03-18 Thread Richard Henderson
On 03/18/2014 04:59 AM, Jakub Jelinek wrote:
 On Mon, Mar 17, 2014 at 03:24:14PM -0400, Vladimir Makarov wrote:
 It is complicated.  There is no guarantee that it is used only for
 addresses.  I need some time to think how to fix it.

 Meanwhile, you *should* commit the patch into the trunk because it
 solves the real problem.  And I can work from this to make changes
 that the new pattern is only used for addresses.

 The patch is absolutely safe for all targets but s390.  There is
 still a tiny possibility that it might result in some problems for
 s390  (now I see only one situation when a pseudo in a subreg
 changed by equiv plus expr needs a reload).  In any case your patch
 solves real numerous failures and can be used as a base for further
 work.

 Thanks for working on this problem, Andreas.  Sorry that I missed
 the PR60501.
 
 BTW, does LRA require that CC isn't clobbered when it uses
 emit_add2_insn? I don't see how it can be guaranteed
 (except perhaps on i?86/x86_64 and maybe a few other targets).

At least in pre-LRA days one *had* to have such an addition, or alternately you
had to leave compare+use as a single insn pattern until post-reload splitting.
 I assume that's really unchanged with LRA...

 emit_add3_insn should be ok (even on s390*?) because recog_memoized
 should (I think) never add clobbers (it calls recog with 0
 as last argument), but gen_add2_insn is a normall addPmode3 insn that
 on many targets clobbers CC.

Sure, but s390 has no choice but to implement the clobber-less add with the
LOAD ADDRESS instruction.

Which is perfectly safe for 64-bit, but for 32-bit will in fact crop that 31st
bit.  Which is perfectly safe so long as we never want to do addition except on
pointers.  Which brings us back to Vlad's point about REG_EQUIV being a
potential hazzard.

I agree with Vlad that we're better off with Andreas' patch than without, since
computing addresses is going to be 99% of what reload/LRA needs to do.

I also agree with Eric that some better commentary would be nice.


r~


Re: [PATCH] BZ60501: Add addptr optab

2014-03-17 Thread Vladimir Makarov

On 2014-03-13, 7:37 AM, Andreas Krebbel wrote:

On 13/03/14 12:25, Richard Biener wrote:

On Thu, Mar 13, 2014 at 12:16 PM, Eric Botcazou ebotca...@adacore.com wrote:

--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -4720,6 +4720,17 @@ Add operand 2 and operand 1, storing the result in
operand 0.  All operands must have mode @var{m}.  This can be used even on
two-address machines, by means of constraints requiring operands 1 and 0 to
be the same location.

+@cindex @code{addptr@var{m}3} instruction pattern
+@item @samp{addptr@var{m}3}
+Like @code{addptr@var{m}3} but does never clobber the condition code.
+It only needs to be defined if @code{add@var{m}3} either sets the
+condition code or address calculations cannot be performed with the
+normal add instructions due to other reasons.  If adds used for
+address calculations and normal adds are not compatible it is required
+to expand a distinct pattern (e.g. using an unspec).  The pattern is
+used by LRA to emit address calculations.  @code{add@var{m}3} is used
+if @code{addptr@var{m}3} is not defined.


I'm a bit skeptical of the address calculations cannot be performed with the
normal add instructions due to other reasons part.  Surely they can be
performed on all architectures supported by GCC as of this writing, otherwise
how would the compiler even work?  And if it's really like @code{add@var{m}3},
why restricting it to addresses, i.e. why calling it @code{addptr@var{m}3}?
Does that come from an implementation constraint on s390 that supports it only
for a subset of the cases supported by @code{add@var{m}3}?


Yeah, isn't it that you want a named pattern like add_nocc for an add
that doesn't clobber flags?

This would suggest that you can use the pattern also for performing a normal 
add in case the
condition code is not needed afterwards but this isn't correct for s390 31 bit 
where an address
calculation is actually something different.  addptr is better I think because 
it is a pattern which
is supposed to be implemented with a load address instruction and the 
middle-end guarantees to use
it only on addresses. (I hope LRA is actually behaving that way). Perhaps 
loadptr or la or
loadaddress would be a better name?



It is complicated.  There is no guarantee that it is used only for 
addresses.  I need some time to think how to fix it.


Meanwhile, you *should* commit the patch into the trunk because it 
solves the real problem.  And I can work from this to make changes that 
the new pattern is only used for addresses.


The patch is absolutely safe for all targets but s390.  There is still a 
tiny possibility that it might result in some problems for s390  (now I 
see only one situation when a pseudo in a subreg changed by equiv plus 
expr needs a reload).  In any case your patch solves real numerous 
failures and can be used as a base for further work.


Thanks for working on this problem, Andreas.  Sorry that I missed the 
PR60501.




Re: [PATCH] BZ60501: Add addptr optab

2014-03-15 Thread Eric Botcazou
  Then you should document that by stating that the pattern is guaranteed
  to be invoked only for addresses (and may not clobber the condition
  code).
 Ok, will do.

Thanks.

  Hoping isn't sufficient IMO here, you need to rename/rework
  emit_add3_insn and possibly stop the compiler if the value it is
  invoked on is not an address.
 Agreed.  Any idea how to check for this?

The head comment of lra_emit_add appears to be saying that it is invoked only 
for addresses; it's clear when it is invoked from base_plus_disp_to_reg but 
it's less clear when it is invoked from lra_emit_move.  So a possible approach 
could be to split lra_emit_move into 2 parts: lra_emit_move with an assertion 
that y isn't a PLUS and lra_emit_address_move with the call to lra_emit_add, 
and review all the callers.

If that's too much work, just copy (part of) the head comment of lra_emit_add 
to that of emit_add3_insn and cross your fingers (yes, the latter action adds 
something over just hoping :-)

-- 
Eric Botcazou


Re: [PATCH] BZ60501: Add addptr optab

2014-03-14 Thread Eric Botcazou
 This would suggest that you can use the pattern also for performing a normal
 add in case the condition code is not needed afterwards but this isn't
 correct for s390 31 bit where an address calculation is actually something
 different.

Then you should document that by stating that the pattern is guaranteed to be 
invoked only for addresses (and may not clobber the condition code).

 addptr is better I think because it is a pattern which is
 supposed to be implemented with a load address instruction and the
 middle-end guarantees to use it only on addresses. (I hope LRA is actually
 behaving that way).

Hoping isn't sufficient IMO here, you need to rename/rework emit_add3_insn and 
possibly stop the compiler if the value it is invoked on is not an address.

-- 
Eric Botcazou


Re: [PATCH] BZ60501: Add addptr optab

2014-03-14 Thread Andreas Krebbel
On 14/03/14 11:02, Eric Botcazou wrote:
 This would suggest that you can use the pattern also for performing a normal
 add in case the condition code is not needed afterwards but this isn't
 correct for s390 31 bit where an address calculation is actually something
 different.
 
 Then you should document that by stating that the pattern is guaranteed to be 
 invoked only for addresses (and may not clobber the condition code).
Ok, will do.

 addptr is better I think because it is a pattern which is
 supposed to be implemented with a load address instruction and the
 middle-end guarantees to use it only on addresses. (I hope LRA is actually
 behaving that way).
 
 Hoping isn't sufficient IMO here, you need to rename/rework emit_add3_insn 
 and 
 possibly stop the compiler if the value it is invoked on is not an address.
Agreed.  Any idea how to check for this?

Bye,

-Andreas-



[PATCH] BZ60501: Add addptr optab

2014-03-13 Thread Andreas Krebbel
Hi,

fixes the LRA problems described in:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60501
and
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57604

Bootstrapped and regtested on s390, s390x, and x86_64.

Ok?

Bye,

-Andreas-

2014-03-13  Andreas Krebbel  andreas.kreb...@de.ibm.com

PR rtl-optimization/60501
* optabs.def (addptr3_optab): New optab.
* optabs.c (gen_addptr3_insn, have_addptr3_insn): New function.
* doc/md.texi (addptrm3): Document new RTL standard expander.
* expr.h (gen_addptr3_insn, have_addptr3_insn): Add prototypes.

* lra.c (emit_add3_insn): Use the addptr pattern if available.

* config/s390/s390.md (addptrdi3, addptrsi3): New expanders.

diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index 76902b5..7d9d1ad 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -5034,6 +5034,57 @@
   [(set_attr op_type  RRer,RXE)
(set_attr type fsimpmode)])
 
+;
+; Pointer add instruction patterns
+;
+
+; This will match *la_64
+(define_expand addptrdi3
+  [(set (match_operand:DI 0 register_operand )
+(plus:DI (match_operand:DI 1 register_operand )
+(match_operand:DI 2 nonmemory_operand )))]
+  TARGET_64BIT
+{
+  HOST_WIDE_INT c = INTVAL (operands[2]);
+
+  if (GET_CODE (operands[2]) == CONST_INT)
+{
+  if (!CONST_OK_FOR_CONSTRAINT_P (c, 'K', K)
+  !CONST_OK_FOR_CONSTRAINT_P (c, 'O', Os))
+{
+ operands[2] = force_const_mem (DImode, operands[2]);
+ operands[2] = force_reg (DImode, operands[2]);
+}
+  else if (!DISP_IN_RANGE (INTVAL (operands[2])))
+operands[2] = force_reg (DImode, operands[2]);
+}
+})
+
+; For 31 bit we have to prevent the generated pattern from matching
+; normal ADDs since la only does a 31 bit add.  This is supposed to
+; match force_la_31.
+(define_expand addptrsi3
+  [(parallel
+[(set (match_operand:SI 0 register_operand )
+ (plus:SI (match_operand:SI 1 register_operand )
+  (match_operand:SI 2 nonmemory_operand )))
+  (use (const_int 0))])]
+  !TARGET_64BIT
+{
+  HOST_WIDE_INT c = INTVAL (operands[2]);
+
+  if (GET_CODE (operands[2]) == CONST_INT)
+{
+  if (!CONST_OK_FOR_CONSTRAINT_P (c, 'K', K)
+  !CONST_OK_FOR_CONSTRAINT_P (c, 'O', Os))
+{
+ operands[2] = force_const_mem (SImode, operands[2]);
+ operands[2] = force_reg (SImode, operands[2]);
+}
+  else if (!DISP_IN_RANGE (INTVAL (operands[2])))
+operands[2] = force_reg (SImode, operands[2]);
+}
+})
 
 ;;
 ;;- Subtract instructions.
diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index 746acc2..972b717 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -4720,6 +4720,17 @@ Add operand 2 and operand 1, storing the result in 
operand 0.  All operands
 must have mode @var{m}.  This can be used even on two-address machines, by
 means of constraints requiring operands 1 and 0 to be the same location.
 
+@cindex @code{addptr@var{m}3} instruction pattern
+@item @samp{addptr@var{m}3}
+Like @code{addptr@var{m}3} but does never clobber the condition code.
+It only needs to be defined if @code{add@var{m}3} either sets the
+condition code or address calculations cannot be performed with the
+normal add instructions due to other reasons.  If adds used for
+address calculations and normal adds are not compatible it is required
+to expand a distinct pattern (e.g. using an unspec).  The pattern is
+used by LRA to emit address calculations.  @code{add@var{m}3} is used
+if @code{addptr@var{m}3} is not defined.
+
 @cindex @code{ssadd@var{m}3} instruction pattern
 @cindex @code{usadd@var{m}3} instruction pattern
 @cindex @code{sub@var{m}3} instruction pattern
diff --git a/gcc/expr.h b/gcc/expr.h
index 5111f06..524da67 100644
--- a/gcc/expr.h
+++ b/gcc/expr.h
@@ -180,10 +180,12 @@ extern void emit_libcall_block (rtx, rtx, rtx, rtx);
Likewise for subtraction and for just copying.  */
 extern rtx gen_add2_insn (rtx, rtx);
 extern rtx gen_add3_insn (rtx, rtx, rtx);
+extern rtx gen_addptr3_insn (rtx, rtx, rtx);
 extern rtx gen_sub2_insn (rtx, rtx);
 extern rtx gen_sub3_insn (rtx, rtx, rtx);
 extern rtx gen_move_insn (rtx, rtx);
 extern int have_add2_insn (rtx, rtx);
+extern int have_addptr3_insn (rtx, rtx, rtx);
 extern int have_sub2_insn (rtx, rtx);
 
 /* Emit a pair of rtl insns to compare two rtx's and to jump
diff --git a/gcc/lra.c b/gcc/lra.c
index 77074e2..e5e81474 100644
--- a/gcc/lra.c
+++ b/gcc/lra.c
@@ -254,6 +254,19 @@ emit_add3_insn (rtx x, rtx y, rtx z)
   rtx insn, last;
 
   last = get_last_insn ();
+  
+  if (have_addptr3_insn (x, y, z))
+{
+  insn = gen_addptr3_insn (x, y, z);
+
+  /* If the target provides an addptr pattern it hopefully does
+for a reason.  So falling back to the normal add would be
+a bug.  */
+  lra_assert (insn != NULL_RTX);
+  emit_insn (insn);
+  return insn;
+}
+
   insn = 

Re: [PATCH] BZ60501: Add addptr optab

2014-03-13 Thread Jakub Jelinek
On Thu, Mar 13, 2014 at 10:24:13AM +0100, Andreas Krebbel wrote:
 --- a/gcc/doc/md.texi
 +++ b/gcc/doc/md.texi
 @@ -4720,6 +4720,17 @@ Add operand 2 and operand 1, storing the result in 
 operand 0.  All operands
  must have mode @var{m}.  This can be used even on two-address machines, by
  means of constraints requiring operands 1 and 0 to be the same location.
  
 +@cindex @code{addptr@var{m}3} instruction pattern
 +@item @samp{addptr@var{m}3}
 +Like @code{addptr@var{m}3} but does never clobber the condition code.

Didn't you mean Like @code{add@var{m}3} here?

 +int
 +have_addptr3_insn (rtx x, rtx y, rtx z)

Missing function comment.

Otherwise looks good to me, but please give Vladimir, Jeff or Eric 24 hours to
comment on it.

Jakub


Re: [PATCH] BZ60501: Add addptr optab

2014-03-13 Thread Eric Botcazou
 --- a/gcc/doc/md.texi
 +++ b/gcc/doc/md.texi
 @@ -4720,6 +4720,17 @@ Add operand 2 and operand 1, storing the result in
 operand 0.  All operands must have mode @var{m}.  This can be used even on
 two-address machines, by means of constraints requiring operands 1 and 0 to
 be the same location.
 
 +@cindex @code{addptr@var{m}3} instruction pattern
 +@item @samp{addptr@var{m}3}
 +Like @code{addptr@var{m}3} but does never clobber the condition code.
 +It only needs to be defined if @code{add@var{m}3} either sets the
 +condition code or address calculations cannot be performed with the
 +normal add instructions due to other reasons.  If adds used for
 +address calculations and normal adds are not compatible it is required
 +to expand a distinct pattern (e.g. using an unspec).  The pattern is
 +used by LRA to emit address calculations.  @code{add@var{m}3} is used
 +if @code{addptr@var{m}3} is not defined.

I'm a bit skeptical of the address calculations cannot be performed with the 
normal add instructions due to other reasons part.  Surely they can be 
performed on all architectures supported by GCC as of this writing, otherwise 
how would the compiler even work?  And if it's really like @code{add@var{m}3}, 
why restricting it to addresses, i.e. why calling it @code{addptr@var{m}3}?  
Does that come from an implementation constraint on s390 that supports it only 
for a subset of the cases supported by @code{add@var{m}3}?

 diff --git a/gcc/lra.c b/gcc/lra.c
 index 77074e2..e5e81474 100644
 --- a/gcc/lra.c
 +++ b/gcc/lra.c
 @@ -254,6 +254,19 @@ emit_add3_insn (rtx x, rtx y, rtx z)
rtx insn, last;
 
last = get_last_insn ();
 +
 +  if (have_addptr3_insn (x, y, z))
 +{
 +  insn = gen_addptr3_insn (x, y, z);
 +
 +  /* If the target provides an addptr pattern it hopefully does
 +  for a reason.  So falling back to the normal add would be
 +  a bug.  */
 +  lra_assert (insn != NULL_RTX);
 +  emit_insn (insn);
 +  return insn;
 +}
 +
insn = emit_insn (gen_rtx_SET (VOIDmode, x,
gen_rtx_PLUS (GET_MODE (y), y, z)));
if (recog_memoized (insn)  0)

Same ambiguity here, emit_add3_insn is not documented as being restricted to 
addresses:

/* Emit insn x = y + z.  Return NULL if we failed to do it.
   Otherwise, return the insn.  We don't use gen_add3_insn as it might
   clobber CC.  */
static rtx
emit_add3_insn (rtx x, rtx y, rtx z)

-- 
Eric Botcazou


Re: [PATCH] BZ60501: Add addptr optab

2014-03-13 Thread Richard Biener
On Thu, Mar 13, 2014 at 12:16 PM, Eric Botcazou ebotca...@adacore.com wrote:
 --- a/gcc/doc/md.texi
 +++ b/gcc/doc/md.texi
 @@ -4720,6 +4720,17 @@ Add operand 2 and operand 1, storing the result in
 operand 0.  All operands must have mode @var{m}.  This can be used even on
 two-address machines, by means of constraints requiring operands 1 and 0 to
 be the same location.

 +@cindex @code{addptr@var{m}3} instruction pattern
 +@item @samp{addptr@var{m}3}
 +Like @code{addptr@var{m}3} but does never clobber the condition code.
 +It only needs to be defined if @code{add@var{m}3} either sets the
 +condition code or address calculations cannot be performed with the
 +normal add instructions due to other reasons.  If adds used for
 +address calculations and normal adds are not compatible it is required
 +to expand a distinct pattern (e.g. using an unspec).  The pattern is
 +used by LRA to emit address calculations.  @code{add@var{m}3} is used
 +if @code{addptr@var{m}3} is not defined.

 I'm a bit skeptical of the address calculations cannot be performed with the
 normal add instructions due to other reasons part.  Surely they can be
 performed on all architectures supported by GCC as of this writing, otherwise
 how would the compiler even work?  And if it's really like @code{add@var{m}3},
 why restricting it to addresses, i.e. why calling it @code{addptr@var{m}3}?
 Does that come from an implementation constraint on s390 that supports it only
 for a subset of the cases supported by @code{add@var{m}3}?

Yeah, isn't it that you want a named pattern like add_nocc for an add
that doesn't clobber flags?

Richard.

 diff --git a/gcc/lra.c b/gcc/lra.c
 index 77074e2..e5e81474 100644
 --- a/gcc/lra.c
 +++ b/gcc/lra.c
 @@ -254,6 +254,19 @@ emit_add3_insn (rtx x, rtx y, rtx z)
rtx insn, last;

last = get_last_insn ();
 +
 +  if (have_addptr3_insn (x, y, z))
 +{
 +  insn = gen_addptr3_insn (x, y, z);
 +
 +  /* If the target provides an addptr pattern it hopefully does
 +  for a reason.  So falling back to the normal add would be
 +  a bug.  */
 +  lra_assert (insn != NULL_RTX);
 +  emit_insn (insn);
 +  return insn;
 +}
 +
insn = emit_insn (gen_rtx_SET (VOIDmode, x,
gen_rtx_PLUS (GET_MODE (y), y, z)));
if (recog_memoized (insn)  0)

 Same ambiguity here, emit_add3_insn is not documented as being restricted to
 addresses:

 /* Emit insn x = y + z.  Return NULL if we failed to do it.
Otherwise, return the insn.  We don't use gen_add3_insn as it might
clobber CC.  */
 static rtx
 emit_add3_insn (rtx x, rtx y, rtx z)

 --
 Eric Botcazou