This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch] fix regrename pass to ensure renamings produce valid insns
- From: Sandra Loosemore <sandra at codesourcery dot com>
- To: Eric Botcazou <ebotcazou at adacore dot com>
- Cc: Chung-Lin Tang <cltang at codesourcery dot com>, <gcc-patches at gcc dot gnu dot org>, Kito Cheng <kito dot cheng at gmail dot com>, Jeff Law <law at redhat dot com>, Marcus Shawcroft <Marcus dot Shawcroft at arm dot com>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>, "Schmidt, Bernd - Code Sourcery" <bernds at codesourcery dot com>
- Date: Tue, 30 Jun 2015 12:58:33 -0600
- Subject: Re: [patch] fix regrename pass to ensure renamings produce valid insns
- Authentication-results: sourceware.org; auth=none
- References: <5581AA41 dot 7010201 at codesourcery dot com> <55922571 dot 2050808 at codesourcery dot com> <5592287F dot 4050203 at codesourcery dot com> <2385820 dot 32MGCG8euH at polaris>
On 06/30/2015 03:06 AM, Eric Botcazou wrote:
I notice the way gcc_assert() is defined in system.h now, the test won't
disappear even when runtime checks are disabled, though you might still
adjust it to avoid any programmer confusion.
It will disappear at run time, see the definition:
/* Include EXPR, so that unused variable warnings do not occur. */
#define gcc_assert(EXPR) ((void)(0 && (EXPR)))
so you really need to use a separate variable.
Oh, yuck -- it never even occurred to me that gcc_assert could be
disabled. I'll bet there are other bugs in GCC due to this very same
problem of depending on its argument being executed for side-effect.
(E.g. take a look at add_stmt_to_eh_lp_fn in tree-eh.c.) Seems like
lousy design to me especially since proper usage doesn't seem to be
documented anywhere.
Anyway, I think the attached patch is what's required to fix the
instance that's my fault. OK? Bernd, if this needs testing, can you help?
-Sandra
2015-06-30 Sandra Loosemore <sandra@codesourcery.com>
gcc/
* config/c6x/c6x.c (try_rename_operands): Do not depend on
gcc_assert evaluating its argument for side-effect.
Index: gcc/config/c6x/c6x.c
===================================================================
--- gcc/config/c6x/c6x.c (revision 225202)
+++ gcc/config/c6x/c6x.c (working copy)
@@ -3450,6 +3450,7 @@ try_rename_operands (rtx_insn *head, rtx
int best_reg, old_reg;
vec<du_head_p> involved_chains = vNULL;
unit_req_table new_reqs;
+ bool ok;
for (i = 0, tmp_mask = op_mask; tmp_mask; i++)
{
@@ -3516,7 +3517,8 @@ try_rename_operands (rtx_insn *head, rtx
best_reg =
find_rename_reg (this_head, super_class, &unavailable, old_reg, true);
- gcc_assert (regrename_do_replace (this_head, best_reg));
+ ok = regrename_do_replace (this_head, best_reg);
+ gcc_assert (ok);
count_unit_reqs (new_reqs, head, PREV_INSN (tail));
merge_unit_reqs (new_reqs);
@@ -3529,7 +3531,10 @@ try_rename_operands (rtx_insn *head, rtx
unit_req_imbalance (reqs), unit_req_imbalance (new_reqs));
}
if (unit_req_imbalance (new_reqs) > unit_req_imbalance (reqs))
- gcc_assert (regrename_do_replace (this_head, old_reg));
+ {
+ ok = regrename_do_replace (this_head, old_reg);
+ gcc_assert (ok);
+ }
else
memcpy (reqs, new_reqs, sizeof (unit_req_table));