| Message ID | 20140313092413.GA14597@bart |
|---|---|
| State | New |
| Headers | show |
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
> --- 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)
On Thu, Mar 13, 2014 at 12:16 PM, Eric Botcazou <ebotcazou@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
On 13/03/14 12:16, Eric Botcazou 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? For S/390 it really is only the condition code clobber why the normal add cannot be used here. Apart from that an add can do everything an load adress can do. I don't have an example for other reasons which might require this. So I can remove it. > 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}? For S/390 31 bit an address calculation cannot be used as a normal add. An la is actually only a 31 bit operation. Bye, -Andreas- > >> 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) >
On 13/03/14 12:25, Richard Biener wrote: > On Thu, Mar 13, 2014 at 12:16 PM, Eric Botcazou <ebotcazou@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? Bye, -Andreas- > > 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 >
> 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.
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-
> > 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 :-)
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 <ebotcazou@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.
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 add<Pmode>3 insn that on many targets clobbers CC. Jakub
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 add<Pmode>3 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~
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" "fsimp<mode>")]) +; +; 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 = emit_insn (gen_rtx_SET (VOIDmode, x, gen_rtx_PLUS (GET_MODE (y), y, z))); if (recog_memoized (insn) < 0) diff --git a/gcc/optabs.c b/gcc/optabs.c index cec25a4..49e704e 100644 --- a/gcc/optabs.c +++ b/gcc/optabs.c @@ -4755,6 +4755,40 @@ have_add2_insn (rtx x, rtx y) return 1; } +/* Generate and return an insn body to add Y to X. */ + +rtx +gen_addptr3_insn (rtx x, rtx y, rtx z) +{ + enum insn_code icode = optab_handler (addptr3_optab, GET_MODE (x)); + + gcc_assert (insn_operand_matches (icode, 0, x)); + gcc_assert (insn_operand_matches (icode, 1, y)); + gcc_assert (insn_operand_matches (icode, 2, z)); + + return GEN_FCN (icode) (x, y, z); +} + +int +have_addptr3_insn (rtx x, rtx y, rtx z) +{ + enum insn_code icode; + + gcc_assert (GET_MODE (x) != VOIDmode); + + icode = optab_handler (addptr3_optab, GET_MODE (x)); + + if (icode == CODE_FOR_nothing) + return 0; + + if (!insn_operand_matches (icode, 0, x) + || !insn_operand_matches (icode, 1, y) + || !insn_operand_matches (icode, 2, z)) + return 0; + + return 1; +} + /* Generate and return an insn body to subtract Y from X. */ rtx diff --git a/gcc/optabs.def b/gcc/optabs.def index decdaf3..9b89740 100644 --- a/gcc/optabs.def +++ b/gcc/optabs.def @@ -191,6 +191,7 @@ OPTAB_D (addv4_optab, "addv$I$a4") OPTAB_D (subv4_optab, "subv$I$a4") OPTAB_D (mulv4_optab, "mulv$I$a4") OPTAB_D (negv3_optab, "negv$I$a3") +OPTAB_D (addptr3_optab, "addptr$a3") OPTAB_D (smul_highpart_optab, "smul$a3_highpart") OPTAB_D (umul_highpart_optab, "umul$a3_highpart")
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.Krebbel@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.