* [patch] Fix failure of ACATS c45503c at -O2 @ 2015-10-20 16:38 Eric Botcazou 2015-10-20 19:04 ` Jeff Law 0 siblings, 1 reply; 11+ messages in thread From: Eric Botcazou @ 2015-10-20 16:38 UTC (permalink / raw) To: gcc-patches [-- Attachment #1: Type: text/plain, Size: 874 bytes --] Hi, this test started to fail recently as the result of the work of Richard S., but the underlying issue had been latent for a long time. It boils down to this excerpt from the VRP1 dump file: Found new range for _9: [0, 12] marking stmt to be not simulated again Visiting statement: _3 = _9 %[fl] _11; Found new range for _3: [0, +INF] which is plain wrong since the sign of FLOOR_MOD_EXPR is the sign of the divisor and not that of the dividend (the latter is for TRUNC_MOD_EXPR). I have attached a fix for mainline and another one for the 4.9/5 branches. Tested on x86_64-suse-linux, OK for all active branches? 2015-10-20 Eric Botcazou <ebotcazou@adacore.com> * fold-const.c (tree_binary_nonnegative_warnv_p) <FLOOR_MOD_EXPR>: Recurse on operand #1 instead of operand #0. <CEIL_MOD_EXPR>: Do not recurse. <ROUND_MOD_EXPR>: Likewise. -- Eric Botcazou [-- Attachment #2: p1.diff --] [-- Type: text/x-patch, Size: 722 bytes --] Index: fold-const.c =================================================================== --- fold-const.c (revision 229022) +++ fold-const.c (working copy) @@ -12982,11 +12982,15 @@ tree_binary_nonnegative_warnv_p (enum tr return RECURSE (op0) && RECURSE (op1); case TRUNC_MOD_EXPR: - case CEIL_MOD_EXPR: - case FLOOR_MOD_EXPR: - case ROUND_MOD_EXPR: + /* The sign of the remainder is that of the dividend. */ return RECURSE (op0); + case FLOOR_MOD_EXPR: + /* The sign of the remainder is that of the divisor. */ + return RECURSE (op1); + + case CEIL_MOD_EXPR: + case ROUND_MOD_EXPR: default: return tree_simple_nonnegative_warnv_p (code, type); } [-- Attachment #3: p2.diff --] [-- Type: text/x-patch, Size: 806 bytes --] Index: fold-const.c =================================================================== --- fold-const.c (revision 228932) +++ fold-const.c (working copy) @@ -14853,11 +14853,17 @@ tree_binary_nonnegative_warnv_p (enum tr strict_overflow_p)); case TRUNC_MOD_EXPR: - case CEIL_MOD_EXPR: - case FLOOR_MOD_EXPR: - case ROUND_MOD_EXPR: + /* The sign of the remainder is that of the dividend. */ return tree_expr_nonnegative_warnv_p (op0, strict_overflow_p); + + case FLOOR_MOD_EXPR: + /* The sign of the remainder is that of the divisor. */ + return tree_expr_nonnegative_warnv_p (op1, + strict_overflow_p); + + case CEIL_MOD_EXPR: + case ROUND_MOD_EXPR: default: return tree_simple_nonnegative_warnv_p (code, type); } ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] Fix failure of ACATS c45503c at -O2 2015-10-20 16:38 [patch] Fix failure of ACATS c45503c at -O2 Eric Botcazou @ 2015-10-20 19:04 ` Jeff Law 2015-10-20 22:10 ` Joseph Myers 2015-10-20 22:30 ` Eric Botcazou 0 siblings, 2 replies; 11+ messages in thread From: Jeff Law @ 2015-10-20 19:04 UTC (permalink / raw) To: Eric Botcazou, gcc-patches On 10/20/2015 10:33 AM, Eric Botcazou wrote: > Hi, > > this test started to fail recently as the result of the work of Richard S., > but the underlying issue had been latent for a long time. It boils down to > this excerpt from the VRP1 dump file: > > Found new range for _9: [0, 12] > marking stmt to be not simulated again > > Visiting statement: > _3 = _9 %[fl] _11; > Found new range for _3: [0, +INF] > > which is plain wrong since the sign of FLOOR_MOD_EXPR is the sign of the > divisor and not that of the dividend (the latter is for TRUNC_MOD_EXPR). > > I have attached a fix for mainline and another one for the 4.9/5 branches. > > Tested on x86_64-suse-linux, OK for all active branches? > > > 2015-10-20 Eric Botcazou <ebotcazou@adacore.com> > > * fold-const.c (tree_binary_nonnegative_warnv_p) <FLOOR_MOD_EXPR>: > Recurse on operand #1 instead of operand #0. > <CEIL_MOD_EXPR>: Do not recurse. > <ROUND_MOD_EXPR>: Likewise. Isn't this a function of the language and in some cases isn't it implementation defined (true for C/C++ until C++11)? Even with that in mind, I think FLOOR_MOD_EXPR and TRUNC_MOD_EXPR are more correct with your patch. I'm just not sure about CEIL & ROUND. Jeff ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] Fix failure of ACATS c45503c at -O2 2015-10-20 19:04 ` Jeff Law @ 2015-10-20 22:10 ` Joseph Myers 2015-10-21 5:57 ` Jeff Law 2015-10-20 22:30 ` Eric Botcazou 1 sibling, 1 reply; 11+ messages in thread From: Joseph Myers @ 2015-10-20 22:10 UTC (permalink / raw) To: Jeff Law; +Cc: Eric Botcazou, gcc-patches On Tue, 20 Oct 2015, Jeff Law wrote: > > 2015-10-20 Eric Botcazou <ebotcazou@adacore.com> > > > > * fold-const.c (tree_binary_nonnegative_warnv_p) <FLOOR_MOD_EXPR>: > > Recurse on operand #1 instead of operand #0. > > <CEIL_MOD_EXPR>: Do not recurse. > > <ROUND_MOD_EXPR>: Likewise. > Isn't this a function of the language and in some cases isn't it > implementation defined (true for C/C++ until C++11)? The language determines *which* *_DIV_EXPR or *_MOD_EXPR is used. The semantics of each of them should be language-independent. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] Fix failure of ACATS c45503c at -O2 2015-10-20 22:10 ` Joseph Myers @ 2015-10-21 5:57 ` Jeff Law 2015-10-21 7:50 ` Eric Botcazou 0 siblings, 1 reply; 11+ messages in thread From: Jeff Law @ 2015-10-21 5:57 UTC (permalink / raw) To: Joseph Myers; +Cc: Eric Botcazou, gcc-patches On 10/20/2015 04:00 PM, Joseph Myers wrote: > On Tue, 20 Oct 2015, Jeff Law wrote: > >>> 2015-10-20 Eric Botcazou <ebotcazou@adacore.com> >>> >>> * fold-const.c (tree_binary_nonnegative_warnv_p) <FLOOR_MOD_EXPR>: >>> Recurse on operand #1 instead of operand #0. >>> <CEIL_MOD_EXPR>: Do not recurse. >>> <ROUND_MOD_EXPR>: Likewise. >> Isn't this a function of the language and in some cases isn't it >> implementation defined (true for C/C++ until C++11)? > > The language determines *which* *_DIV_EXPR or *_MOD_EXPR is used. The > semantics of each of them should be language-independent. You're, of course, correct. So the only question is whether or not the CEIL_MOD_EXPR and ROUND_MOD_EXPR bits are right. I'm confident the change to FLOOR_MOD_EXPR is right. Do we have any reasonable way to test CEIL_MOD_EXPR & ROUND_MOD_EXPR? Jeff ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] Fix failure of ACATS c45503c at -O2 2015-10-21 5:57 ` Jeff Law @ 2015-10-21 7:50 ` Eric Botcazou 2015-10-21 9:56 ` Richard Biener 0 siblings, 1 reply; 11+ messages in thread From: Eric Botcazou @ 2015-10-21 7:50 UTC (permalink / raw) To: Jeff Law; +Cc: gcc-patches, Joseph Myers > So the only question is whether or not the CEIL_MOD_EXPR and > ROUND_MOD_EXPR bits are right. I'm confident the change to > FLOOR_MOD_EXPR is right. OK. > Do we have any reasonable way to test CEIL_MOD_EXPR & ROUND_MOD_EXPR? Note that the patch makes the function punt on those 2 so it can do no harm. The sign of CEIL_MOD_EXPR is predictable (opposite of that of the divisor) but you cannot use that in the context; and finally the sign of ROUND_MOD_EXPR isn't predictable. I can add a few more comments: Index: fold-const.c =================================================================== --- fold-const.c (revision 229022) +++ fold-const.c (working copy) @@ -12982,11 +12982,18 @@ tree_binary_nonnegative_warnv_p (enum tr return RECURSE (op0) && RECURSE (op1); case TRUNC_MOD_EXPR: - case CEIL_MOD_EXPR: - case FLOOR_MOD_EXPR: - case ROUND_MOD_EXPR: + /* The sign of the remainder is that of the dividend. */ return RECURSE (op0); + case FLOOR_MOD_EXPR: + /* The sign of the remainder is that of the divisor. */ + return RECURSE (op1); + + case CEIL_MOD_EXPR: + /* The sign of the remainder is the opposite of that of the divisor, + but this cannot be used in this context. */ + case ROUND_MOD_EXPR: + /* The sign of the remainder is not predictable. */ default: return tree_simple_nonnegative_warnv_p (code, type); } -- Eric Botcazou ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] Fix failure of ACATS c45503c at -O2 2015-10-21 7:50 ` Eric Botcazou @ 2015-10-21 9:56 ` Richard Biener 2015-10-21 13:15 ` Eric Botcazou 0 siblings, 1 reply; 11+ messages in thread From: Richard Biener @ 2015-10-21 9:56 UTC (permalink / raw) To: Eric Botcazou; +Cc: Jeff Law, GCC Patches, Joseph Myers On Wed, Oct 21, 2015 at 9:24 AM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> So the only question is whether or not the CEIL_MOD_EXPR and >> ROUND_MOD_EXPR bits are right. I'm confident the change to >> FLOOR_MOD_EXPR is right. > > OK. > >> Do we have any reasonable way to test CEIL_MOD_EXPR & ROUND_MOD_EXPR? > > Note that the patch makes the function punt on those 2 so it can do no harm. > The sign of CEIL_MOD_EXPR is predictable (opposite of that of the divisor) but > you cannot use that in the context; and finally the sign of ROUND_MOD_EXPR > isn't predictable. > > I can add a few more comments: Maybe add the comments to tree.def instead. > Index: fold-const.c > =================================================================== > --- fold-const.c (revision 229022) > +++ fold-const.c (working copy) > @@ -12982,11 +12982,18 @@ tree_binary_nonnegative_warnv_p (enum tr > return RECURSE (op0) && RECURSE (op1); > > case TRUNC_MOD_EXPR: > - case CEIL_MOD_EXPR: > - case FLOOR_MOD_EXPR: > - case ROUND_MOD_EXPR: > + /* The sign of the remainder is that of the dividend. */ > return RECURSE (op0); > > + case FLOOR_MOD_EXPR: > + /* The sign of the remainder is that of the divisor. */ > + return RECURSE (op1); > + > + case CEIL_MOD_EXPR: > + /* The sign of the remainder is the opposite of that of the divisor, > + but this cannot be used in this context. */ > + case ROUND_MOD_EXPR: > + /* The sign of the remainder is not predictable. */ > default: > return tree_simple_nonnegative_warnv_p (code, type); > } > > > -- > Eric Botcazou ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] Fix failure of ACATS c45503c at -O2 2015-10-21 9:56 ` Richard Biener @ 2015-10-21 13:15 ` Eric Botcazou 2015-10-21 13:18 ` Richard Biener 2015-10-21 21:11 ` Richard Sandiford 0 siblings, 2 replies; 11+ messages in thread From: Eric Botcazou @ 2015-10-21 13:15 UTC (permalink / raw) To: Richard Biener; +Cc: gcc-patches, Jeff Law, Joseph Myers [-- Attachment #1: Type: text/plain, Size: 488 bytes --] > Maybe add the comments to tree.def instead. Good idea, revised patch attached. * tree.def (CEIL_DIV_EXPR, FLOOR_DIV_EXPR, ROUND_DIV_EXPR): Tweak comments. (TRUNC_MOD_EXPR, CEIL_MOD_EXPR, FLOOR_MOD_EXPR, ROUND_MOD_EXPR): Add comments on sign of the result. * fold-const.c (tree_binary_nonnegative_warnv_p) <FLOOR_MOD_EXPR>: Recurse on operand #1 instead of operand #0. <CEIL_MOD_EXPR>: Do not recurse. <ROUND_MOD_EXPR>: Likewise. -- Eric Botcazou [-- Attachment #2: p.diff --] [-- Type: text/x-patch, Size: 2259 bytes --] Index: tree.def =================================================================== --- tree.def (revision 229123) +++ tree.def (working copy) @@ -685,19 +685,27 @@ DEFTREECODE (MULT_HIGHPART_EXPR, "mult_h /* Division for integer result that rounds the quotient toward zero. */ DEFTREECODE (TRUNC_DIV_EXPR, "trunc_div_expr", tcc_binary, 2) -/* Division for integer result that rounds the quotient toward infinity. */ +/* Division for integer result that rounds it toward plus infinity. */ DEFTREECODE (CEIL_DIV_EXPR, "ceil_div_expr", tcc_binary, 2) -/* Division for integer result that rounds toward minus infinity. */ +/* Division for integer result that rounds it toward minus infinity. */ DEFTREECODE (FLOOR_DIV_EXPR, "floor_div_expr", tcc_binary, 2) -/* Division for integer result that rounds toward nearest integer. */ +/* Division for integer result that rounds it toward nearest integer. */ DEFTREECODE (ROUND_DIV_EXPR, "round_div_expr", tcc_binary, 2) -/* Four kinds of remainder that go with the four kinds of division. */ +/* Four kinds of remainder that go with the four kinds of division: */ + +/* The sign of the remainder is that of the dividend. */ DEFTREECODE (TRUNC_MOD_EXPR, "trunc_mod_expr", tcc_binary, 2) + +/* The sign of the remainder is the opposite of that of the divisor. */ DEFTREECODE (CEIL_MOD_EXPR, "ceil_mod_expr", tcc_binary, 2) + +/* The sign of the remainder is that of the divisor. */ DEFTREECODE (FLOOR_MOD_EXPR, "floor_mod_expr", tcc_binary, 2) + +/* The sign of the remainder is not predictable. */ DEFTREECODE (ROUND_MOD_EXPR, "round_mod_expr", tcc_binary, 2) /* Division for real result. */ Index: fold-const.c =================================================================== --- fold-const.c (revision 229123) +++ fold-const.c (working copy) @@ -12929,11 +12929,13 @@ tree_binary_nonnegative_warnv_p (enum tr return RECURSE (op0) && RECURSE (op1); case TRUNC_MOD_EXPR: - case CEIL_MOD_EXPR: - case FLOOR_MOD_EXPR: - case ROUND_MOD_EXPR: return RECURSE (op0); + case FLOOR_MOD_EXPR: + return RECURSE (op1); + + case CEIL_MOD_EXPR: + case ROUND_MOD_EXPR: default: return tree_simple_nonnegative_warnv_p (code, type); } ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] Fix failure of ACATS c45503c at -O2 2015-10-21 13:15 ` Eric Botcazou @ 2015-10-21 13:18 ` Richard Biener 2015-10-21 21:11 ` Richard Sandiford 1 sibling, 0 replies; 11+ messages in thread From: Richard Biener @ 2015-10-21 13:18 UTC (permalink / raw) To: Eric Botcazou; +Cc: GCC Patches, Jeff Law, Joseph Myers On Wed, Oct 21, 2015 at 3:06 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> Maybe add the comments to tree.def instead. > > Good idea, revised patch attached. Ok. Thanks, Richard. > > * tree.def (CEIL_DIV_EXPR, FLOOR_DIV_EXPR, ROUND_DIV_EXPR): Tweak > comments. > (TRUNC_MOD_EXPR, CEIL_MOD_EXPR, FLOOR_MOD_EXPR, ROUND_MOD_EXPR): > Add comments on sign of the result. > * fold-const.c (tree_binary_nonnegative_warnv_p) <FLOOR_MOD_EXPR>: > Recurse on operand #1 instead of operand #0. > <CEIL_MOD_EXPR>: Do not recurse. > <ROUND_MOD_EXPR>: Likewise. > > -- > Eric Botcazou ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] Fix failure of ACATS c45503c at -O2 2015-10-21 13:15 ` Eric Botcazou 2015-10-21 13:18 ` Richard Biener @ 2015-10-21 21:11 ` Richard Sandiford 1 sibling, 0 replies; 11+ messages in thread From: Richard Sandiford @ 2015-10-21 21:11 UTC (permalink / raw) To: Eric Botcazou; +Cc: gcc-patches Eric Botcazou <ebotcazou@adacore.com> writes: >> Maybe add the comments to tree.def instead. > > Good idea, revised patch attached. > > > * tree.def (CEIL_DIV_EXPR, FLOOR_DIV_EXPR, ROUND_DIV_EXPR): Tweak > comments. > (TRUNC_MOD_EXPR, CEIL_MOD_EXPR, FLOOR_MOD_EXPR, ROUND_MOD_EXPR): > Add comments on sign of the result. > * fold-const.c (tree_binary_nonnegative_warnv_p) <FLOOR_MOD_EXPR>: > Recurse on operand #1 instead of operand #0. > <CEIL_MOD_EXPR>: Do not recurse. > <ROUND_MOD_EXPR>: Likewise. Thanks for fixing this and sorry for the breakage. I do normally test Ada but must have used the wrong baseline. Thanks, Richard ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] Fix failure of ACATS c45503c at -O2 2015-10-20 19:04 ` Jeff Law 2015-10-20 22:10 ` Joseph Myers @ 2015-10-20 22:30 ` Eric Botcazou 2015-10-20 22:55 ` Joseph Myers 1 sibling, 1 reply; 11+ messages in thread From: Eric Botcazou @ 2015-10-20 22:30 UTC (permalink / raw) To: Jeff Law; +Cc: gcc-patches > Isn't this a function of the language and in some cases isn't it > implementation defined (true for C/C++ until C++11)? I don't think that C/C++ use FLOOR_MOD_EXPR, only Ada does AFAIK. In any case, I don't see how this can be implementation-defined given: /* Division for integer result that rounds the quotient toward zero. */ DEFTREECODE (TRUNC_DIV_EXPR, "trunc_div_expr", tcc_binary, 2) /* Division for integer result that rounds the quotient toward infinity. */ DEFTREECODE (CEIL_DIV_EXPR, "ceil_div_expr", tcc_binary, 2) /* Division for integer result that rounds toward minus infinity. */ DEFTREECODE (FLOOR_DIV_EXPR, "floor_div_expr", tcc_binary, 2) /* Division for integer result that rounds toward nearest integer. */ DEFTREECODE (ROUND_DIV_EXPR, "round_div_expr", tcc_binary, 2) /* Four kinds of remainder that go with the four kinds of division. */ DEFTREECODE (TRUNC_MOD_EXPR, "trunc_mod_expr", tcc_binary, 2) DEFTREECODE (CEIL_MOD_EXPR, "ceil_mod_expr", tcc_binary, 2) DEFTREECODE (FLOOR_MOD_EXPR, "floor_mod_expr", tcc_binary, 2) DEFTREECODE (ROUND_MOD_EXPR, "round_mod_expr", tcc_binary, 2) This isn't immediate but, given the definition of TRUNC_DIV_EXPR, you can prove that the sign of TRUNC_MOD_EXPR is that of the dividend; similarly, given the definition of FLOOR_DIV_EXPR, you can prove that the sign of FLOOR_MOD_EXPR is that of the divisor; given the definition of CEIL_DIV_EXPR, you can prove that the sign of CEIL_MOD_EXPR is the opposite of that of the divisor (but you cannot use the latter result in this context). -- Eric Botcazou ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [patch] Fix failure of ACATS c45503c at -O2 2015-10-20 22:30 ` Eric Botcazou @ 2015-10-20 22:55 ` Joseph Myers 0 siblings, 0 replies; 11+ messages in thread From: Joseph Myers @ 2015-10-20 22:55 UTC (permalink / raw) To: Eric Botcazou; +Cc: Jeff Law, gcc-patches On Wed, 21 Oct 2015, Eric Botcazou wrote: > > Isn't this a function of the language and in some cases isn't it > > implementation defined (true for C/C++ until C++11)? > > I don't think that C/C++ use FLOOR_MOD_EXPR, only Ada does AFAIK. In any > case, I don't see how this can be implementation-defined given: And, given that RTL semantics are as well-defined and target-independent as GENERIC / GIMPLE semantics here, a corollary is that the MMIX option -mknuthdiv is fundamentally ill-conceived (affecting as it does the semantics of RTL operations whose semantics are assumed by the rest of the compiler to be target-independent). -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2015-10-21 21:09 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-10-20 16:38 [patch] Fix failure of ACATS c45503c at -O2 Eric Botcazou 2015-10-20 19:04 ` Jeff Law 2015-10-20 22:10 ` Joseph Myers 2015-10-21 5:57 ` Jeff Law 2015-10-21 7:50 ` Eric Botcazou 2015-10-21 9:56 ` Richard Biener 2015-10-21 13:15 ` Eric Botcazou 2015-10-21 13:18 ` Richard Biener 2015-10-21 21:11 ` Richard Sandiford 2015-10-20 22:30 ` Eric Botcazou 2015-10-20 22:55 ` Joseph Myers
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).