[Bug c/86617] [6/7/8/9 Regression] Volatile qualifier is ignored sometimes for unsigned char
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
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
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
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
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
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
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
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; }