[Bug c/86617] [6/7/8/9 Regression] Volatile qualifier is ignored sometimes for unsigned char

2018-07-23 Thread edlinger at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86617

--- Comment #8 from Bernd Edlinger  ---
Author: edlinger
Date: Mon Jul 23 13:23:51 2018
New Revision: 262933

URL: https://gcc.gnu.org/viewcvs?rev=262933&root=gcc&view=rev
Log:
gcc:
2018-07-23  Bernd Edlinger  

PR c/86617
* genmatch.c (dt_operand::gen_match_op): Avoid folding volatile values.

testsuite:
2018-07-23  Bernd Edlinger  

PR c/86617
* gcc.dg/pr86617.c: New test.

Added:
trunk/gcc/testsuite/gcc.dg/pr86617.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/genmatch.c
trunk/gcc/testsuite/ChangeLog

[Bug c/86617] [6/7/8/9 Regression] Volatile qualifier is ignored sometimes for unsigned char

2018-07-23 Thread bernd.edlinger at hotmail dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86617

--- Comment #7 from Bernd Edlinger  ---
Yes. Sure.

[Bug c/86617] [6/7/8/9 Regression] Volatile qualifier is ignored sometimes for unsigned char

2018-07-23 Thread rguenther at suse dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86617

--- Comment #6 from rguenther at suse dot de  ---
On Mon, 23 Jul 2018, bernd.edlinger at hotmail dot de wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86617
> 
> --- Comment #5 from Bernd Edlinger  ---
> (In reply to Richard Biener from comment #3)
> > but I guess that doesn't work because the counting is missing.  OTOH
> > two same SAVE_EXPRs () are not operand_equal_p but SAVE_EXPRs have
> > TREE_SIDE_EFFECTS set but we can safely handle SAVE_EXPR + SAVE_EXPR.
> > 
> 
> shouldn't that be fixed in operand_equal_p ?

Probably.  Can you test your patch and post it?

[Bug c/86617] [6/7/8/9 Regression] Volatile qualifier is ignored sometimes for unsigned char

2018-07-23 Thread bernd.edlinger at hotmail dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86617

--- Comment #5 from Bernd Edlinger  ---
(In reply to Richard Biener from comment #3)
> but I guess that doesn't work because the counting is missing.  OTOH
> two same SAVE_EXPRs () are not operand_equal_p but SAVE_EXPRs have
> TREE_SIDE_EFFECTS set but we can safely handle SAVE_EXPR + SAVE_EXPR.
> 

shouldn't that be fixed in operand_equal_p ?

[Bug c/86617] [6/7/8/9 Regression] Volatile qualifier is ignored sometimes for unsigned char

2018-07-23 Thread bernd.edlinger at hotmail dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86617

--- Comment #4 from Bernd Edlinger  ---
this comment in match.pd made me look at operand_equal_p:

/* Simplify x - x.
   This is unsafe for certain floats even in non-IEEE formats.
   In IEEE, it is unsafe because it does wrong for NaNs.
   Also note that operand_equal_p is always false if an operand
   is volatile.  */

[Bug c/86617] [6/7/8/9 Regression] Volatile qualifier is ignored sometimes for unsigned char

2018-07-23 Thread rguenth at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86617

Richard Biener  changed:

   What|Removed |Added

   Priority|P3  |P2
 CC||rguenth at gcc dot gnu.org
   Target Milestone|--- |6.5

--- Comment #3 from Richard Biener  ---
Oops.  But really side-effects should have been handled via

  /* Search for captures not used in the result expression and
dependent
 on TREE_SIDE_EFFECTS emit omit_one_operand.  */
  for (int i = 0; i < s->capture_max + 1; ++i)
{
  if (cinfo.info[i].same_as != (unsigned)i)
continue;
  if (!cinfo.info[i].force_no_side_effects_p
  && !cinfo.info[i].expr_p
  && cinfo.info[i].result_use_count == 0)
{
  fprintf_indent (f, indent,
  "if (TREE_SIDE_EFFECTS (captures[%d]))\n",
  i);
  fprintf_indent (f, indent + 2,
  "res = build2_loc (loc, COMPOUND_EXPR, type,
"
  "fold_ignored_result (captures[%d]),
res);\n",
  i);
}
}

but I guess that doesn't work because the counting is missing.  OTOH
two same SAVE_EXPRs () are not operand_equal_p but SAVE_EXPRs have
TREE_SIDE_EFFECTS set but we can safely handle SAVE_EXPR + SAVE_EXPR.

So I think a fix needs more thinking.  Obviously simply never treating
things with side-effects the same is a conservative fix.

[Bug c/86617] [6/7/8/9 Regression] Volatile qualifier is ignored sometimes for unsigned char

2018-07-22 Thread bernd.edlinger at hotmail dot de
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86617

Bernd Edlinger  changed:

   What|Removed |Added

 CC||bernd.edlinger at hotmail dot 
de

--- Comment #2 from Bernd Edlinger  ---
Oh, interesting:

Index: genmatch.c
===
--- genmatch.c  (revision 262904)
+++ genmatch.c  (working copy)
@@ -2748,12 +2748,14 @@
   char match_opname[20];
   match_dop->get_name (match_opname);
   if (value_match)
-fprintf_indent (f, indent, "if (%s == %s || operand_equal_p (%s, %s,
0))\n",
-   opname, match_opname, opname, match_opname);
+fprintf_indent (f, indent, "if ((%s == %s && ! TREE_SIDE_EFFECTS (%s)) "
+   "|| operand_equal_p (%s, %s, 0))\n",
+   opname, match_opname, opname, opname, match_opname);
   else
-fprintf_indent (f, indent, "if (%s == %s || (operand_equal_p (%s, %s, 0) "
+fprintf_indent (f, indent, "if ((%s == %s && ! TREE_SIDE_EFFECTS (%s)) "
+   "|| (operand_equal_p (%s, %s, 0) "
"&& types_match (%s, %s)))\n",
-   opname, match_opname, opname, match_opname,
+   opname, match_opname, opname, opname, match_opname,
opname, match_opname);
   fprintf_indent (f, indent + 2, "{\n");
   return 1;

[Bug c/86617] [6/7/8/9 Regression] Volatile qualifier is ignored sometimes for unsigned char

2018-07-21 Thread amonakov at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86617

Alexander Monakov  changed:

   What|Removed |Added

   Keywords||wrong-code
 Status|UNCONFIRMED |NEW
   Last reconfirmed||2018-07-21
 CC||amonakov at gcc dot gnu.org
Summary|Volatile qualifier is   |[6/7/8/9 Regression]
   |ignored sometimes for   |Volatile qualifier is
   |unsigned char   |ignored sometimes for
   ||unsigned char
 Ever confirmed|0   |1

--- Comment #1 from Alexander Monakov  ---
Confirmed, 'unsigned short' is similarly mishandled, but not wider integer
types. gcc-4.9 got this right. Appears like over-eager folding in the frontend:
in the .original dump I get

{
  u8 = u8 * 2;
  u8 = u8, 0;
}