* [PATCH, asmcons] Fix PR rtl-optimization/89313: ICE in process_alt_operands, at lra-constraints.c:2962
@ 2019-02-21 7:08 Peter Bergner
2019-03-06 14:57 ` Segher Boessenkool
2019-03-25 20:42 ` Jeff Law
0 siblings, 2 replies; 6+ messages in thread
From: Peter Bergner @ 2019-02-21 7:08 UTC (permalink / raw)
To: GCC Patches; +Cc: Segher Boessenkool, Alan Modra
PR89313 exposes a bug in the asmcons pass where it replaces input operands
with matching constraints with their associated output operands, as well as
all other uses of its pseudo registers. This is normally fine. However, if
the matched output operand is marked as early clobber, then we cannot replace
the uses of 'input' that do not have matching constraints, since they by
definition conflict with the early clobber output operand and could be
clobbered if assigned to the same register as the output operand.
The patch below fixes the bug by only doing the input pseudo replacement
if the output operand is not early clobber or the input operand is known
to be a matching constraint.
This passed bootstrap and regression testing with no regressions on
both x86_64-linux and powerpc64le-linux. Ok for mainline?
Peter
gcc/
PR rtl-optimization/89313
* function.c (matching_constraint_num): New static function.
(match_asm_constraints_1): Use it. Fixup white space and comment.
Don't replace inputs with non-matching constraints which conflict
with early clobber outputs.
gcc/testsuite/
PR rtl-optimization/89313
* gcc.dg/pr89313.c: New test.
Index: gcc/function.c
===================================================================
--- gcc/function.c (revision 268883)
+++ gcc/function.c (working copy)
@@ -6395,6 +6395,36 @@ make_pass_thread_prologue_and_epilogue (
}
\f
+/* If CONSTRAINT is a matching constraint, then return its number.
+ Otherwise, return -1. */
+
+static int
+matching_constraint_num (const char *constraint)
+{
+ int match;
+
+ if (*constraint == '%')
+ constraint++;
+
+ switch (*constraint)
+ {
+ case '0': case '1': case '2': case '3': case '4':
+ case '5': case '6': case '7': case '8': case '9':
+ {
+ char *end;
+ match = strtoul (constraint, &end, 10);
+ if (end == constraint)
+ match = -1;
+ break;
+ }
+
+ default:
+ match = -1;
+ break;
+ }
+ return match;
+}
+
/* This mini-pass fixes fall-out from SSA in asm statements that have
in-out constraints. Say you start with
@@ -6453,14 +6483,10 @@ match_asm_constraints_1 (rtx_insn *insn,
rtx input, output;
rtx_insn *insns;
const char *constraint = ASM_OPERANDS_INPUT_CONSTRAINT (op, i);
- char *end;
int match, j;
- if (*constraint == '%')
- constraint++;
-
- match = strtoul (constraint, &end, 10);
- if (end == constraint)
+ match = matching_constraint_num (constraint);
+ if (match < 0)
continue;
gcc_assert (match < noutputs);
@@ -6477,14 +6503,14 @@ match_asm_constraints_1 (rtx_insn *insn,
/* We can't do anything if the output is also used as input,
as we're going to overwrite it. */
for (j = 0; j < ninputs; j++)
- if (reg_overlap_mentioned_p (output, RTVEC_ELT (inputs, j)))
+ if (reg_overlap_mentioned_p (output, RTVEC_ELT (inputs, j)))
break;
if (j != ninputs)
continue;
/* Avoid changing the same input several times. For
asm ("" : "=mr" (out1), "=mr" (out2) : "0" (in), "1" (in));
- only change in once (to out1), rather than changing it
+ only change it once (to out1), rather than changing it
first to out1 and afterwards to out2. */
if (i > 0)
{
@@ -6502,6 +6528,9 @@ match_asm_constraints_1 (rtx_insn *insn,
end_sequence ();
emit_insn_before (insns, insn);
+ constraint = ASM_OPERANDS_OUTPUT_CONSTRAINT(SET_SRC(p_sets[match]));
+ bool early_clobber_p = strchr (constraint, '&') != NULL;
+
/* Now replace all mentions of the input with output. We can't
just replace the occurrence in inputs[i], as the register might
also be used in some other input (or even in an address of an
@@ -6523,7 +6552,14 @@ match_asm_constraints_1 (rtx_insn *insn,
value, but different pseudos) where we formerly had only one.
With more complicated asms this might lead to reload failures
which wouldn't have happen without this pass. So, iterate over
- all operands and replace all occurrences of the register used. */
+ all operands and replace all occurrences of the register used.
+
+ However, if one or more of the 'input' uses have a non-matching
+ constraint and the matched output operand is an early clobber
+ operand, then do not replace the input operand, since by definition
+ it conflicts with the output operand and cannot share the same
+ register. See PR89313 for details. */
+
for (j = 0; j < noutputs; j++)
if (!rtx_equal_p (SET_DEST (p_sets[j]), input)
&& reg_overlap_mentioned_p (input, SET_DEST (p_sets[j])))
@@ -6531,8 +6567,13 @@ match_asm_constraints_1 (rtx_insn *insn,
input, output);
for (j = 0; j < ninputs; j++)
if (reg_overlap_mentioned_p (input, RTVEC_ELT (inputs, j)))
- RTVEC_ELT (inputs, j) = replace_rtx (RTVEC_ELT (inputs, j),
- input, output);
+ {
+ if (!early_clobber_p
+ || match == matching_constraint_num
+ (ASM_OPERANDS_INPUT_CONSTRAINT (op, j)))
+ RTVEC_ELT (inputs, j) = replace_rtx (RTVEC_ELT (inputs, j),
+ input, output);
+ }
changed = true;
}
Index: gcc/testsuite/gcc.dg/pr89313.c
===================================================================
--- gcc/testsuite/gcc.dg/pr89313.c (nonexistent)
+++ gcc/testsuite/gcc.dg/pr89313.c (working copy)
@@ -0,0 +1,26 @@
+/* PR rtl-optimization/89313 */
+/* { dg-do compile { target aarch64*-*-* arm*-*-* i?86-*-* powerpc*-*-* s390*-*-* x86_64-*-* } } */
+/* { dg-options "-O2" } */
+
+#if defined (__aarch64__)
+# define REG "x0"
+#elif defined (__arm__)
+# define REG "r0"
+#elif defined (__i386__)
+# define REG "%eax"
+#elif defined (__powerpc__)
+# define REG "r3"
+#elif defined (__s390__)
+# define REG "0"
+#elif defined (__x86_64__)
+# define REG "rax"
+#endif
+
+long
+bug (long arg)
+{
+ register long output asm (REG);
+ long input = arg;
+ asm ("blah %0, %1, %2" : "=&r" (output) : "r" (input), "0" (input));
+ return output;
+}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, asmcons] Fix PR rtl-optimization/89313: ICE in process_alt_operands, at lra-constraints.c:2962
2019-02-21 7:08 [PATCH, asmcons] Fix PR rtl-optimization/89313: ICE in process_alt_operands, at lra-constraints.c:2962 Peter Bergner
@ 2019-03-06 14:57 ` Segher Boessenkool
2019-03-06 15:06 ` Peter Bergner
2019-03-25 20:42 ` Jeff Law
1 sibling, 1 reply; 6+ messages in thread
From: Segher Boessenkool @ 2019-03-06 14:57 UTC (permalink / raw)
To: Peter Bergner; +Cc: GCC Patches, Alan Modra
Hi Peter,
On Wed, Feb 20, 2019 at 09:19:58PM -0600, Peter Bergner wrote:
> PR89313 exposes a bug in the asmcons pass where it replaces input operands
> with matching constraints with their associated output operands, as well as
> all other uses of its pseudo registers. This is normally fine. However, if
> the matched output operand is marked as early clobber, then we cannot replace
> the uses of 'input' that do not have matching constraints, since they by
> definition conflict with the early clobber output operand and could be
> clobbered if assigned to the same register as the output operand.
>
> The patch below fixes the bug by only doing the input pseudo replacement
> if the output operand is not early clobber or the input operand is known
> to be a matching constraint.
>
> This passed bootstrap and regression testing with no regressions on
> both x86_64-linux and powerpc64le-linux. Ok for mainline?
Looks fine to me, but I cannot approve it of course. Some trivial
comments:
> +/* If CONSTRAINT is a matching constraint, then return its number.
> + Otherwise, return -1. */
> +
> +static int
> +matching_constraint_num (const char *constraint)
> +{
> + int match;
> +
> + if (*constraint == '%')
> + constraint++;
> +
> + switch (*constraint)
> + {
> + case '0': case '1': case '2': case '3': case '4':
> + case '5': case '6': case '7': case '8': case '9':
> + {
> + char *end;
> + match = strtoul (constraint, &end, 10);
> + if (end == constraint)
> + match = -1;
This condition is always false, because we have at least one digit.
> + break;
> + }
> +
> + default:
> + match = -1;
> + break;
> + }
> + return match;
> +}
Which means you can write it as just
static int
matching_constraint_num (const char *constraint)
{
if (*constraint == '%')
constraint++;
if (IN_RANGE (*constraint, '0', '9'))
return strtoul (constraint, NULL, 10);
return -1;
}
Segher
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, asmcons] Fix PR rtl-optimization/89313: ICE in process_alt_operands, at lra-constraints.c:2962
2019-03-06 14:57 ` Segher Boessenkool
@ 2019-03-06 15:06 ` Peter Bergner
2019-03-06 17:15 ` Peter Bergner
0 siblings, 1 reply; 6+ messages in thread
From: Peter Bergner @ 2019-03-06 15:06 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: GCC Patches, Alan Modra
On 3/6/19 8:47 AM, Segher Boessenkool wrote:
> Which means you can write it as just
>
> static int
> matching_constraint_num (const char *constraint)
> {
> if (*constraint == '%')
> constraint++;
>
> if (IN_RANGE (*constraint, '0', '9'))
> return strtoul (constraint, NULL, 10);
>
> return -1;
> }
Ok, changed. Thanks!
Peter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, asmcons] Fix PR rtl-optimization/89313: ICE in process_alt_operands, at lra-constraints.c:2962
2019-03-06 15:06 ` Peter Bergner
@ 2019-03-06 17:15 ` Peter Bergner
0 siblings, 0 replies; 6+ messages in thread
From: Peter Bergner @ 2019-03-06 17:15 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: GCC Patches, Alan Modra
On 3/6/19 8:56 AM, Peter Bergner wrote:
> On 3/6/19 8:47 AM, Segher Boessenkool wrote:
>> Which means you can write it as just
>>
>> static int
>> matching_constraint_num (const char *constraint)
>> {
>> if (*constraint == '%')
>> constraint++;
>>
>> if (IN_RANGE (*constraint, '0', '9'))
>> return strtoul (constraint, NULL, 10);
>>
>> return -1;
>> }
>
> Ok, changed. Thanks!
...and re-bootstrapping and regtesting were clean with that change.
Peter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, asmcons] Fix PR rtl-optimization/89313: ICE in process_alt_operands, at lra-constraints.c:2962
2019-02-21 7:08 [PATCH, asmcons] Fix PR rtl-optimization/89313: ICE in process_alt_operands, at lra-constraints.c:2962 Peter Bergner
2019-03-06 14:57 ` Segher Boessenkool
@ 2019-03-25 20:42 ` Jeff Law
2019-03-27 17:11 ` Peter Bergner
1 sibling, 1 reply; 6+ messages in thread
From: Jeff Law @ 2019-03-25 20:42 UTC (permalink / raw)
To: Peter Bergner, GCC Patches; +Cc: Segher Boessenkool, Alan Modra
On 2/20/19 8:19 PM, Peter Bergner wrote:
> PR89313 exposes a bug in the asmcons pass where it replaces input operands
> with matching constraints with their associated output operands, as well as
> all other uses of its pseudo registers. This is normally fine. However, if
> the matched output operand is marked as early clobber, then we cannot replace
> the uses of 'input' that do not have matching constraints, since they by
> definition conflict with the early clobber output operand and could be
> clobbered if assigned to the same register as the output operand.
>
> The patch below fixes the bug by only doing the input pseudo replacement
> if the output operand is not early clobber or the input operand is known
> to be a matching constraint.
>
> This passed bootstrap and regression testing with no regressions on
> both x86_64-linux and powerpc64le-linux. Ok for mainline?
>
> Peter
>
>
> gcc/
> PR rtl-optimization/89313
> * function.c (matching_constraint_num): New static function.
> (match_asm_constraints_1): Use it. Fixup white space and comment.
> Don't replace inputs with non-matching constraints which conflict
> with early clobber outputs.
>
> gcc/testsuite/
> PR rtl-optimization/89313
> * gcc.dg/pr89313.c: New test.
OK.
jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH, asmcons] Fix PR rtl-optimization/89313: ICE in process_alt_operands, at lra-constraints.c:2962
2019-03-25 20:42 ` Jeff Law
@ 2019-03-27 17:11 ` Peter Bergner
0 siblings, 0 replies; 6+ messages in thread
From: Peter Bergner @ 2019-03-27 17:11 UTC (permalink / raw)
To: Jeff Law; +Cc: GCC Patches, Segher Boessenkool, Alan Modra
On 3/25/19 3:36 PM, Jeff Law wrote:
> On 2/20/19 8:19 PM, Peter Bergner wrote:
>> gcc/
>> PR rtl-optimization/89313
>> * function.c (matching_constraint_num): New static function.
>> (match_asm_constraints_1): Use it. Fixup white space and comment.
>> Don't replace inputs with non-matching constraints which conflict
>> with early clobber outputs.
>>
>> gcc/testsuite/
>> PR rtl-optimization/89313
>> * gcc.dg/pr89313.c: New test.
> OK.
I did another round of bootstrap and regtesting, since I was on vacation
and trunk has changed. The testing was still clean, so it's committed now.
Thanks!
Peter
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-03-27 17:02 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-21 7:08 [PATCH, asmcons] Fix PR rtl-optimization/89313: ICE in process_alt_operands, at lra-constraints.c:2962 Peter Bergner
2019-03-06 14:57 ` Segher Boessenkool
2019-03-06 15:06 ` Peter Bergner
2019-03-06 17:15 ` Peter Bergner
2019-03-25 20:42 ` Jeff Law
2019-03-27 17:11 ` Peter Bergner
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).