* [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 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
* 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
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).