This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Disallow side-effects in inline asm operands unless (a new) _ constraint modifier is used (PR middle-end/44492)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: gcc-patches at gcc dot gnu dot org
- Date: Fri, 11 Jun 2010 14:00:13 -0400
- Subject: [PATCH] Disallow side-effects in inline asm operands unless (a new) _ constraint modifier is used (PR middle-end/44492)
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
Hi!
As discussed in the PR, currently auto-inc-dec.c happily propagates
{PRE,POST}_{INC,DEC_MODIFY} side-effects into inline asm memory operands
when constraints like "g" or "m" are used.
Unfortunately that means a lot of inline asms out there work by purse
accident - if a mem operand can have a side-effect, special case
needs to be done on the inline asm pattern side. In particular, it has
to be used exactly once in an instruction that is capable of handling
side-effects and on many targets needs to be accompanied by special
modifier too (e.g. on ppc/ppc64 it needs to be used in insn like
lwz%U1 %0,%1, if the %U1 part is omitted, code is silently miscompiled).
A lot of inline asms I've grepped for don't use "m" or "=m" operands
anywhere, many look e.g. like
asm ("... %2 ... " : "=m" (*p) : "m" (*p), "r" (p));
and the "m" and "=m" operands are there just to tell GCC that the insn
(might) reads or writes that memory. If the memory operand with
side-effects isn't used at all, the side-effects don't happen, if it is used
more than once, they happen multiple times.
The following patch disallows side-effects in inline asm operands unless
specifically allowed in source code ( _ constraint modifier says this
operand obeys all the rules that it can contain side-effects).
Bootstrapped/regtested on x86_64-linux and i686-linux (yeah, I know, they
aren't AUTO_INC_DEC) and tested on powerpc64-linux cross.
Ok for trunk?
2010-06-10 Jakub Jelinek <jakub@redhat.com>
PR middle-end/44492
* recog.h (struct recog_data): Add is_asm field.
* recog.c (asm_operand_ok, constrain_operands): Handle _ constraint
modifier. If it is not present and operand contains
{PRE,POST}_{INC,DEC,MODIFY}, return 0.
(extract_insn): Initialize recog_data.is_asm.
(preprocess_constraints): Handle _ constraint modifier.
* ira-costs.c (record_reg_classes): Likewise.
* ira-lives.c (single_reg_class): Likewise.
* stmt.c (parse_output_constraint, parse_input_constraint): Likewise.
* postreload.c (reload_cse_simplify_operands): Likewise.
* reload.c (find_reloads): Likewise.
* reload1.c (maybe_fix_stack_asms): Likewise.
* doc/md.texi (Modifiers): Document _ constraint modifier.
* g++.dg/torture/pr44492.C: New test.
--- gcc/reload1.c.jj 2010-06-10 19:32:01.000000000 +0200
+++ gcc/reload1.c 2010-06-11 10:00:12.000000000 +0200
@@ -1414,6 +1414,7 @@ maybe_fix_stack_asms (void)
case '>': case 'V': case 'o': case '&': case 'E': case 'F':
case 's': case 'i': case 'n': case 'X': case 'I': case 'J':
case 'K': case 'L': case 'M': case 'N': case 'O': case 'P':
+ case '_':
case TARGET_MEM_CONSTRAINT:
break;
--- gcc/recog.h.jj 2010-05-13 12:21:21.000000000 +0200
+++ gcc/recog.h 2010-06-11 10:05:09.000000000 +0200
@@ -230,6 +230,9 @@ struct recog_data
/* The number of alternatives in the constraints for the insn. */
char n_alternatives;
+ /* True if insn is ASM_OPERANDS. */
+ bool is_asm;
+
/* Specifies whether an insn alternative is enabled using the
`enabled' attribute in the insn pattern definition. For back
ends not using the `enabled' attribute the array fields are
--- gcc/postreload.c.jj 2010-06-10 19:32:00.000000000 +0200
+++ gcc/postreload.c 2010-06-11 10:00:11.000000000 +0200
@@ -537,7 +537,7 @@ reload_cse_simplify_operands (rtx insn,
{
case '=': case '+': case '?':
case '#': case '&': case '!':
- case '*': case '%':
+ case '*': case '%': case '_':
case '0': case '1': case '2': case '3': case '4':
case '5': case '6': case '7': case '8': case '9':
case '<': case '>': case 'V': case 'o':
--- gcc/recog.c.jj 2010-06-10 19:32:00.000000000 +0200
+++ gcc/recog.c 2010-06-11 10:14:55.000000000 +0200
@@ -1601,6 +1601,9 @@ int
asm_operand_ok (rtx op, const char *constraint, const char **constraints)
{
int result = 0;
+#ifdef AUTO_INC_DEC
+ bool incdec_ok = false;
+#endif
/* Use constrain_operands after reload. */
gcc_assert (!reload_completed);
@@ -1608,7 +1611,7 @@ asm_operand_ok (rtx op, const char *cons
/* Empty constraint string is the same as "X,...,X", i.e. X for as
many alternatives as required to match the other operands. */
if (*constraint == '\0')
- return 1;
+ result = 1;
while (*constraint)
{
@@ -1629,6 +1632,12 @@ asm_operand_ok (rtx op, const char *cons
case '?':
break;
+ case '_':
+#ifdef AUTO_INC_DEC
+ incdec_ok = true;
+#endif
+ break;
+
case '0': case '1': case '2': case '3': case '4':
case '5': case '6': case '7': case '8': case '9':
/* If caller provided constraints pointer, look up
@@ -1814,6 +1823,23 @@ asm_operand_ok (rtx op, const char *cons
return 0;
}
+#ifdef AUTO_INC_DEC
+ /* For operands without _ constraint modifier reject side-effects. */
+ if (!incdec_ok && result && MEM_P (op))
+ switch (GET_CODE (XEXP (op, 0)))
+ {
+ case PRE_INC:
+ case POST_INC:
+ case PRE_DEC:
+ case POST_DEC:
+ case PRE_MODIFY:
+ case POST_MODIFY:
+ return 0;
+ default:
+ break;
+ }
+#endif
+
return result;
}
@@ -2039,6 +2065,7 @@ extract_insn (rtx insn)
recog_data.n_operands = 0;
recog_data.n_alternatives = 0;
recog_data.n_dups = 0;
+ recog_data.is_asm = false;
switch (GET_CODE (body))
{
@@ -2085,6 +2112,7 @@ extract_insn (rtx insn)
while (*p)
recog_data.n_alternatives += (*p++ == ',');
}
+ recog_data.is_asm = true;
break;
}
fatal_insn_not_found (insn);
@@ -2198,6 +2226,7 @@ preprocess_constraints (void)
case 's': case 'i': case 'n':
case 'I': case 'J': case 'K': case 'L':
case 'M': case 'N': case 'O': case 'P':
+ case '_':
/* These don't say anything we care about. */
break;
@@ -2396,7 +2425,7 @@ constrain_operands (int strict)
break;
case '?': case '!': case '*': case '%':
- case '=': case '+':
+ case '=': case '+': case '_':
break;
case '#':
@@ -2699,6 +2728,29 @@ constrain_operands (int strict)
= recog_data.operand[funny_match[funny_match_index].this_op];
}
+#ifdef AUTO_INC_DEC
+ /* For operands without _ constraint modifier reject
+ side-effects. */
+ if (recog_data.is_asm)
+ {
+ for (opno = 0; opno < recog_data.n_operands; opno++)
+ if (MEM_P (recog_data.operand[opno]))
+ switch (GET_CODE (XEXP (recog_data.operand[opno], 0)))
+ {
+ case PRE_INC:
+ case POST_INC:
+ case PRE_DEC:
+ case POST_DEC:
+ case PRE_MODIFY:
+ case POST_MODIFY:
+ if (strchr (recog_data.constraints[opno], '_') == 0)
+ return 0;
+ break;
+ default:
+ break;
+ }
+ }
+#endif
return 1;
}
}
--- gcc/stmt.c.jj 2010-06-10 19:32:01.000000000 +0200
+++ gcc/stmt.c 2010-06-11 10:00:11.000000000 +0200
@@ -369,7 +369,7 @@ parse_output_constraint (const char **co
case 'E': case 'F': case 'G': case 'H':
case 's': case 'i': case 'n':
case 'I': case 'J': case 'K': case 'L': case 'M':
- case 'N': case 'O': case 'P': case ',':
+ case 'N': case 'O': case 'P': case ',': case '_':
break;
case '0': case '1': case '2': case '3': case '4':
@@ -465,7 +465,7 @@ parse_input_constraint (const char **con
break;
case '<': case '>':
- case '?': case '!': case '*': case '#':
+ case '?': case '!': case '*': case '#': case '_':
case 'E': case 'F': case 'G': case 'H':
case 's': case 'i': case 'n':
case 'I': case 'J': case 'K': case 'L': case 'M':
--- gcc/ira-costs.c.jj 2010-06-10 19:32:01.000000000 +0200
+++ gcc/ira-costs.c 2010-06-11 10:00:11.000000000 +0200
@@ -398,7 +398,7 @@ record_reg_classes (int n_alts, int n_op
case '?':
alt_cost += 2;
- case '!': case '#': case '&':
+ case '!': case '#': case '&': case '_':
case '0': case '1': case '2': case '3': case '4':
case '5': case '6': case '7': case '8': case '9':
break;
--- gcc/reload.c.jj 2010-06-10 19:32:00.000000000 +0200
+++ gcc/reload.c 2010-06-11 10:00:11.000000000 +0200
@@ -3106,7 +3106,7 @@ find_reloads (rtx insn, int replace, int
c = '\0';
break;
- case '=': case '+': case '*':
+ case '=': case '+': case '*': case '_':
break;
case '%':
--- gcc/doc/md.texi.jj 2010-06-11 09:38:04.000000000 +0200
+++ gcc/doc/md.texi 2010-06-11 14:37:13.000000000 +0200
@@ -1595,6 +1595,21 @@ Says that the following character should
register preferences. @samp{*} has no effect on the meaning of the
constraint as a constraint, and no effect on reloading.
+@cindex @samp{_} in constraint
+@item _
+Means that this operand in inline asm can contain side-effects.
+By default side-effects in operands of inline asm are disallowed,
+because there is no guarantee that the operands are used exactly once
+for the inline asm in an instruction that can update the side-effects
+(pre- or post-increment or decrement of a register used in memory
+operand addressing).
+When @samp{_} is added, the operand must be used exactly once in an
+instruction that will do the update and any target specific means
+of ensuring that the side-effects happen in an instruction are present.
+E.g. on PowerPC that means using the operand with @samp{_} constraint
+modifier only in load or store instructions together with @samp{%U}@var{N}
+suffix for the instruction, etc.
+
@ifset INTERNALS
Here is an example: the 68000 has an instruction to sign-extend a
halfword in a data register, and can also sign-extend a value by
--- gcc/ira-lives.c.jj 2010-06-10 19:32:00.000000000 +0200
+++ gcc/ira-lives.c 2010-06-11 10:00:11.000000000 +0200
@@ -640,6 +640,7 @@ single_reg_class (const char *constraint
case '%':
case '!':
case '?':
+ case '_':
break;
case 'i':
if (CONSTANT_P (op)
--- gcc/testsuite/g++.dg/torture/pr44492.C.jj 2010-06-11 10:00:12.000000000 +0200
+++ gcc/testsuite/g++.dg/torture/pr44492.C 2010-06-11 10:00:12.000000000 +0200
@@ -0,0 +1,31 @@
+// PR middle-end/44492
+// { dg-do run }
+
+struct T { unsigned long p; };
+struct S { T a, b, c; unsigned d; };
+
+__attribute__((noinline))
+void
+bar (const T &x, const T &y)
+{
+ if (x.p != 0x2348 || y.p != 0x2346)
+ __builtin_abort ();
+}
+
+__attribute__((noinline))
+void
+foo (S &s, T e)
+{
+ unsigned long a = e.p;
+ unsigned long b = s.b.p;
+ __asm__ volatile ("" : : "rm" (a), "rm" (b));
+ bar (e, s.b);
+}
+
+int
+main ()
+{
+ S s = { { 0x2345 }, { 0x2346 }, { 0x2347 }, 6 };
+ T t = { 0x2348 };
+ foo (s, t);
+}
Jakub