public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/94524] New: wrong code with vector modulo operation
@ 2020-04-07 19:54 zsojka at seznam dot cz
2020-04-07 21:07 ` [Bug tree-optimization/94524] [8/9/10 Regression] wrong code with vector modulo operation since r0-117580 jakub at gcc dot gnu.org
` (8 more replies)
0 siblings, 9 replies; 10+ messages in thread
From: zsojka at seznam dot cz @ 2020-04-07 19:54 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94524
Bug ID: 94524
Summary: wrong code with vector modulo operation
Product: gcc
Version: 10.0
Status: UNCONFIRMED
Keywords: wrong-code
Severity: normal
Priority: P3
Component: tree-optimization
Assignee: unassigned at gcc dot gnu.org
Reporter: zsojka at seznam dot cz
Target Milestone: ---
Host: x86_64-pc-linux-gnu
Created attachment 48232
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48232&action=edit
reduced testcase
Output:
$ x86_64-pc-linux-gnu-gcc -O testcase.c
$ ./a.out
Aborted
I am not sure sure the testcase has defined behavior, but I checked
https://gcc.gnu.org/onlinedocs/gcc/Integers-implementation.html#Integers-implementation
and https://gcc.gnu.org/onlinedocs/gcc/Vector-Extensions.html and I didn't find
any indication that vector operations should behave differently than scalars.
Behavior is the same with -fwrapv and neither -ftrapv or -fsanitize=undefined
don't report any problem.
$ x86_64-pc-linux-gnu-gcc -v
Using built-in specs.
COLLECT_GCC=/repo/gcc-trunk/binary-latest-amd64/bin/x86_64-pc-linux-gnu-gcc
COLLECT_LTO_WRAPPER=/repo/gcc-trunk/binary-trunk-r10-7581-20200407082749-gbee27152f7e-checking-yes-rtl-df-extra-nobootstrap-amd64/bin/../libexec/gcc/x86_64-pc-linux-gnu/10.0.1/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /repo/gcc-trunk//configure --enable-languages=c,c++
--enable-valgrind-annotations --disable-nls --enable-checking=yes,rtl,df,extra
--disable-bootstrap --with-cloog --with-ppl --with-isl
--build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu
--target=x86_64-pc-linux-gnu --with-ld=/usr/bin/x86_64-pc-linux-gnu-ld
--with-as=/usr/bin/x86_64-pc-linux-gnu-as --disable-libstdcxx-pch
--prefix=/repo/gcc-trunk//binary-trunk-r10-7581-20200407082749-gbee27152f7e-checking-yes-rtl-df-extra-nobootstrap-amd64
Thread model: posix
Supported LTO compression algorithms: zlib zstd
gcc version 10.0.1 20200407 (experimental) (GCC)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/94524] [8/9/10 Regression] wrong code with vector modulo operation since r0-117580
2020-04-07 19:54 [Bug tree-optimization/94524] New: wrong code with vector modulo operation zsojka at seznam dot cz
@ 2020-04-07 21:07 ` jakub at gcc dot gnu.org
2020-04-07 22:12 ` jakub at gcc dot gnu.org
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-04-07 21:07 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94524
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|wrong code with vector |[8/9/10 Regression] wrong
|modulo operation |code with vector modulo
| |operation since r0-117580
CC| |jakub at gcc dot gnu.org
Target Milestone|--- |8.5
Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org
Status|UNCONFIRMED |ASSIGNED
Ever confirmed|0 |1
Last reconfirmed| |2020-04-07
--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with my r0-117580-g4ee4c52c64cc1eeda53aae6e221b5b1bd9bd7421 so I
believe this should work correctly in 4.7 (and 4.6 didn't accept it).
The vect pattern code tried to follow what the expander does, but in that case
there is match.pd rule that canonicalizes x % -C into x % C. Will need to dig
up if something in the expansion tweaks the sign of the divisor or if it has
the same bug as the vect pattern.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/94524] [8/9/10 Regression] wrong code with vector modulo operation since r0-117580
2020-04-07 19:54 [Bug tree-optimization/94524] New: wrong code with vector modulo operation zsojka at seznam dot cz
2020-04-07 21:07 ` [Bug tree-optimization/94524] [8/9/10 Regression] wrong code with vector modulo operation since r0-117580 jakub at gcc dot gnu.org
@ 2020-04-07 22:12 ` jakub at gcc dot gnu.org
2020-04-08 6:32 ` rguenth at gcc dot gnu.org
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-04-07 22:12 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94524
--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
4527 /* n rem d = n rem -d */
4528 if (rem_flag && d < 0)
4529 {
4530 d = abs_d;
4531 op1 = gen_int_mode (abs_d, int_mode);
4532 }
in expmed.c indeed canonicalizes this.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/94524] [8/9/10 Regression] wrong code with vector modulo operation since r0-117580
2020-04-07 19:54 [Bug tree-optimization/94524] New: wrong code with vector modulo operation zsojka at seznam dot cz
2020-04-07 21:07 ` [Bug tree-optimization/94524] [8/9/10 Regression] wrong code with vector modulo operation since r0-117580 jakub at gcc dot gnu.org
2020-04-07 22:12 ` jakub at gcc dot gnu.org
@ 2020-04-08 6:32 ` rguenth at gcc dot gnu.org
2020-04-08 7:05 ` jakub at gcc dot gnu.org
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: rguenth at gcc dot gnu.org @ 2020-04-08 6:32 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94524
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Priority|P3 |P2
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/94524] [8/9/10 Regression] wrong code with vector modulo operation since r0-117580
2020-04-07 19:54 [Bug tree-optimization/94524] New: wrong code with vector modulo operation zsojka at seznam dot cz
` (2 preceding siblings ...)
2020-04-08 6:32 ` rguenth at gcc dot gnu.org
@ 2020-04-08 7:05 ` jakub at gcc dot gnu.org
2020-04-08 9:55 ` jakub at gcc dot gnu.org
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-04-08 7:05 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94524
--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Both the tree-vect-patterns.c and tree-vect-generic.c code contains the same.
Except that the latter which handles modulo with a VECTOR_CST divisor, where we
properly divide by the absolute value, but when computing the modulo, we
actually use the original VECTOR_CST to multiply again.
So,
--- gcc/tree-vect-generic.c.jj 2020-01-12 11:54:38.518381650 +0100
+++ gcc/tree-vect-generic.c 2020-04-08 09:02:56.465316469 +0200
@@ -478,6 +478,7 @@ expand_vector_divmod (gimple_stmt_iterat
{
bool use_pow2 = true;
bool has_vector_shift = true;
+ bool use_abs_op1 = false;
int mode = -1, this_mode;
int pre_shift = -1, post_shift;
unsigned int nunits = nunits_for_known_piecewise_op (type);
@@ -618,7 +619,10 @@ expand_vector_divmod (gimple_stmt_iterat
/* n rem d = n rem -d */
if (code == TRUNC_MOD_EXPR && d < 0)
- d = abs_d;
+ {
+ d = abs_d;
+ use_abs_op1 = true;
+ }
else if (abs_d == HOST_WIDE_INT_1U << (prec - 1))
{
/* This case is not handled correctly below. */
@@ -899,6 +903,23 @@ expand_vector_divmod (gimple_stmt_iterat
if (op == unknown_optab
|| optab_handler (op, TYPE_MODE (type)) == CODE_FOR_nothing)
return NULL_TREE;
+ if (use_abs_op1)
+ {
+ tree_vector_builder elts;
+ if (!elts.new_unary_operation (type, op1, false))
+ return NULL_TREE;
+ unsigned int count = elts.encoded_nelts ();
+ for (unsigned int i = 0; i < count; ++i)
+ {
+ tree elem1 = VECTOR_CST_ELT (op1, i);
+
+ tree elt = const_unop (ABS_EXPR, TREE_TYPE (elem1), elem1);
+ if (elt == NULL_TREE)
+ return NULL_TREE;
+ elts.quick_push (elt);
+ }
+ op1 = elts.build ();
+ }
tem = gimplify_build2 (gsi, MULT_EXPR, type, cur_op, op1);
op = optab_for_tree_code (MINUS_EXPR, type, optab_default);
if (op == unknown_optab
should fix this (the last hunk because const_binop apparently doesn't handle
ABS_EXPR on VECTOR_CST :(, and it is probably not appropriate time to add it
now).
That said, I'll need to verify also what we do for modulo with the signed
minimum divisor (either all elts or just one).
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/94524] [8/9/10 Regression] wrong code with vector modulo operation since r0-117580
2020-04-07 19:54 [Bug tree-optimization/94524] New: wrong code with vector modulo operation zsojka at seznam dot cz
` (3 preceding siblings ...)
2020-04-08 7:05 ` jakub at gcc dot gnu.org
@ 2020-04-08 9:55 ` jakub at gcc dot gnu.org
2020-04-08 19:22 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-04-08 9:55 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94524
--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 48239
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48239&action=edit
gcc10-pr94524.patch
Full untested fix.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/94524] [8/9/10 Regression] wrong code with vector modulo operation since r0-117580
2020-04-07 19:54 [Bug tree-optimization/94524] New: wrong code with vector modulo operation zsojka at seznam dot cz
` (4 preceding siblings ...)
2020-04-08 9:55 ` jakub at gcc dot gnu.org
@ 2020-04-08 19:22 ` cvs-commit at gcc dot gnu.org
2020-04-08 19:24 ` [Bug tree-optimization/94524] [8/9 " jakub at gcc dot gnu.org
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-04-08 19:22 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94524
--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:f52eb4f988992d393c69ee4ab76f236dced80e36
commit r10-7639-gf52eb4f988992d393c69ee4ab76f236dced80e36
Author: Jakub Jelinek <jakub@redhat.com>
Date: Wed Apr 8 21:22:05 2020 +0200
vect: Fix up lowering of TRUNC_MOD_EXPR by negative constant [PR94524]
The first testcase below is miscompiled, because for the division part
of the lowering we canonicalize negative divisors to their absolute value
(similarly how expmed.c canonicalizes it), but when multiplying the
division
result back by the VECTOR_CST, we use the original constant, which can
contain negative divisors.
Fixed by computing ABS_EXPR of the VECTOR_CST. Unfortunately, fold-const.c
doesn't support const_unop (ABS_EXPR, VECTOR_CST) and I think it is too
late
in GCC 10 cycle to add it now.
Furthermore, while modulo by most negative constant happens to return the
right value, it does that only by invoking UB in the IL, because
we then expand division by that 1U+INT_MAX and say for INT_MIN % INT_MIN
compute the division as -1, and then multiply by INT_MIN, which is signed
integer overflow. We in theory could do the computation in unsigned vector
types instead, but is it worth bothering. People that are doing % INT_MIN
are either testing for standard conformance, or doing something wrong.
So, I've also added punting on % INT_MIN, both in vect lowering and vect
pattern recognition (we punt already for / INT_MIN).
2020-04-08 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/94524
* tree-vect-generic.c (expand_vector_divmod): If any elt of op1 is
negative for signed TRUNC_MOD_EXPR, multiply with absolute value of
op1 rather than op1 itself at the end. Punt for signed modulo by
most negative constant.
* tree-vect-patterns.c (vect_recog_divmod_pattern): Punt for signed
modulo by most negative constant.
* gcc.c-torture/execute/pr94524-1.c: New test.
* gcc.c-torture/execute/pr94524-2.c: New test.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/94524] [8/9 Regression] wrong code with vector modulo operation since r0-117580
2020-04-07 19:54 [Bug tree-optimization/94524] New: wrong code with vector modulo operation zsojka at seznam dot cz
` (5 preceding siblings ...)
2020-04-08 19:22 ` cvs-commit at gcc dot gnu.org
@ 2020-04-08 19:24 ` jakub at gcc dot gnu.org
2020-09-16 19:20 ` cvs-commit at gcc dot gnu.org
2020-09-17 17:32 ` jakub at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-04-08 19:24 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94524
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|[8/9/10 Regression] wrong |[8/9 Regression] wrong code
|code with vector modulo |with vector modulo
|operation since r0-117580 |operation since r0-117580
--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk so far.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/94524] [8/9 Regression] wrong code with vector modulo operation since r0-117580
2020-04-07 19:54 [Bug tree-optimization/94524] New: wrong code with vector modulo operation zsojka at seznam dot cz
` (6 preceding siblings ...)
2020-04-08 19:24 ` [Bug tree-optimization/94524] [8/9 " jakub at gcc dot gnu.org
@ 2020-09-16 19:20 ` cvs-commit at gcc dot gnu.org
2020-09-17 17:32 ` jakub at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-09-16 19:20 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94524
--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:780b2ab071391495690141c61604370d6cf7af49
commit r9-8877-g780b2ab071391495690141c61604370d6cf7af49
Author: Jakub Jelinek <jakub@redhat.com>
Date: Wed Apr 8 21:22:05 2020 +0200
vect: Fix up lowering of TRUNC_MOD_EXPR by negative constant [PR94524]
The first testcase below is miscompiled, because for the division part
of the lowering we canonicalize negative divisors to their absolute value
(similarly how expmed.c canonicalizes it), but when multiplying the
division
result back by the VECTOR_CST, we use the original constant, which can
contain negative divisors.
Fixed by computing ABS_EXPR of the VECTOR_CST. Unfortunately, fold-const.c
doesn't support const_unop (ABS_EXPR, VECTOR_CST) and I think it is too
late
in GCC 10 cycle to add it now.
Furthermore, while modulo by most negative constant happens to return the
right value, it does that only by invoking UB in the IL, because
we then expand division by that 1U+INT_MAX and say for INT_MIN % INT_MIN
compute the division as -1, and then multiply by INT_MIN, which is signed
integer overflow. We in theory could do the computation in unsigned vector
types instead, but is it worth bothering. People that are doing % INT_MIN
are either testing for standard conformance, or doing something wrong.
So, I've also added punting on % INT_MIN, both in vect lowering and vect
pattern recognition (we punt already for / INT_MIN).
2020-04-08 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/94524
* tree-vect-generic.c (expand_vector_divmod): If any elt of op1 is
negative for signed TRUNC_MOD_EXPR, multiply with absolute value of
op1 rather than op1 itself at the end. Punt for signed modulo by
most negative constant.
* tree-vect-patterns.c (vect_recog_divmod_pattern): Punt for signed
modulo by most negative constant.
* gcc.c-torture/execute/pr94524-1.c: New test.
* gcc.c-torture/execute/pr94524-2.c: New test.
(cherry picked from commit f52eb4f988992d393c69ee4ab76f236dced80e36)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Bug tree-optimization/94524] [8/9 Regression] wrong code with vector modulo operation since r0-117580
2020-04-07 19:54 [Bug tree-optimization/94524] New: wrong code with vector modulo operation zsojka at seznam dot cz
` (7 preceding siblings ...)
2020-09-16 19:20 ` cvs-commit at gcc dot gnu.org
@ 2020-09-17 17:32 ` jakub at gcc dot gnu.org
8 siblings, 0 replies; 10+ messages in thread
From: jakub at gcc dot gnu.org @ 2020-09-17 17:32 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94524
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution|--- |FIXED
--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 8.5 in r8-10486-g7146b8fd63e5107f1cf896df92fbaed99aa5ac0d and for
9.4+ by the above commit too.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-09-17 17:32 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-07 19:54 [Bug tree-optimization/94524] New: wrong code with vector modulo operation zsojka at seznam dot cz
2020-04-07 21:07 ` [Bug tree-optimization/94524] [8/9/10 Regression] wrong code with vector modulo operation since r0-117580 jakub at gcc dot gnu.org
2020-04-07 22:12 ` jakub at gcc dot gnu.org
2020-04-08 6:32 ` rguenth at gcc dot gnu.org
2020-04-08 7:05 ` jakub at gcc dot gnu.org
2020-04-08 9:55 ` jakub at gcc dot gnu.org
2020-04-08 19:22 ` cvs-commit at gcc dot gnu.org
2020-04-08 19:24 ` [Bug tree-optimization/94524] [8/9 " jakub at gcc dot gnu.org
2020-09-16 19:20 ` cvs-commit at gcc dot gnu.org
2020-09-17 17:32 ` jakub at gcc dot gnu.org
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).