* [patch] PR inline-asm/55934
@ 2013-01-17 23:45 Steven Bosscher
2013-01-18 0:00 ` Jakub Jelinek
2013-01-19 0:15 ` Vladimir Makarov
0 siblings, 2 replies; 9+ messages in thread
From: Steven Bosscher @ 2013-01-17 23:45 UTC (permalink / raw)
To: Vladimir Makarov; +Cc: GCC Patches, Jakub Jelinek
Hello Vlad,
Attached is my attempt to fix PR55934, an error recovery issue in LRA
with incorrect constraints in an asm.
I'm not 100% sure this is all correct (especially the LRA insn data
invalidating in lra-assigns.c) but it appears to fix the PR without
introducing test suite failures.
Can you please review the patch?
Thanks,
Ciao!
Steven
gcc/
PR inline-asm/55934
* lra-assigns.c (assign_by_spills): Throw away the pattern of asms
that have operands with impossible constraints.
Add a FIXME for a speed-up opportunity.
* lra-constraints.c (process_alt_operands): Verify that a class
selected from constraints on asms is valid for the operand mode.
(curr_insn_transform): Remove incorrect comment.
testsuite/
PR inline-asm/55934
* gcc.target/i386/pr55934.c: New test.
Index: lra-assigns.c
===================================================================
--- lra-assigns.c (revision 195104)
+++ lra-assigns.c (working copy)
@@ -1240,6 +1240,24 @@ assign_by_spills (void)
asm_p = true;
error_for_asm (insn,
"%<asm%> operand has impossible constraints");
+ /* Avoid further trouble with this insn.
+ For asm goto, instead of fixing up all the edges
+ just clear the template and clear input operands
+ (asm goto doesn't have any output operands). */
+ if (JUMP_P (insn))
+ {
+ rtx asm_op = extract_asm_operands (PATTERN (insn));
+ ASM_OPERANDS_TEMPLATE (asm_op) = ggc_strdup ("");
+ ASM_OPERANDS_INPUT_VEC (asm_op) = rtvec_alloc (0);
+ ASM_OPERANDS_INPUT_CONSTRAINT_VEC (asm_op) =
rtvec_alloc (0);
+ lra_invalidate_insn_data (insn);
+ lra_update_insn_recog_data (insn);
+ }
+ else
+ {
+ PATTERN (insn) = gen_rtx_USE (VOIDmode, const0_rtx);
+ lra_set_insn_deleted (insn);
+ }
}
}
lra_assert (asm_p);
@@ -1263,6 +1281,9 @@ assign_by_spills (void)
bitmap_ior_into (&changed_insns,
&lra_reg_info[sorted_pseudos[i]].insn_bitmap);
}
+
+ /* FIXME: Look up the changed insns in the cached LRA insn data using
+ an EXECUTE_IF_SET_IN_BITMAP over changed_insns. */
FOR_EACH_BB (bb)
FOR_BB_INSNS (bb, insn)
if (bitmap_bit_p (&changed_insns, INSN_UID (insn)))
Index: lra-constraints.c
===================================================================
--- lra-constraints.c (revision 195104)
+++ lra-constraints.c (working copy)
@@ -1847,11 +1847,27 @@ process_alt_operands (int only_alternati
int const_to_mem = 0;
bool no_regs_p;
+ /* If this alternative asks for a specific reg class, see if there
+ is at least one allocatable register in that class. */
no_regs_p
= (this_alternative == NO_REGS
|| (hard_reg_set_subset_p
(reg_class_contents[this_alternative],
lra_no_alloc_regs)));
+
+ /* For asms, verify that the class for this alternative
is possible
+ for the mode that is specified. */
+ if (!no_regs_p && REG_P (op) && INSN_CODE (curr_insn) < 0)
+ {
+ int i;
+ for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
+ if (HARD_REGNO_MODE_OK (i, mode)
+ && in_hard_reg_set_p
(reg_class_contents[this_alternative], mode, i))
+ break;
+ if (i == FIRST_PSEUDO_REGISTER)
+ winreg = false;
+ }
+
/* If this operand accepts a register, and if the
register class has at least one allocatable register,
then this operand can be reloaded. */
@@ -2742,10 +2758,6 @@ curr_insn_transform (void)
swap_operands (commutative);
}
- /* The operands don't meet the constraints. goal_alt describes the
- alternative that we could reach by reloading the fewest operands.
- Reload so as to fit it. */
-
if (! alt_p && ! sec_mem_p)
{
/* No alternative works with reloads?? */
Index: testsuite/gcc.target/i386/pr55934.c
===================================================================
--- testsuite/gcc.target/i386/pr55934.c (revision 0)
+++ testsuite/gcc.target/i386/pr55934.c (revision 0)
@@ -0,0 +1,10 @@
+/* PR inline-asm/55934 */
+/* { dg-do compile } */
+/* { dg-require-effective-target sse } */
+_Complex float
+foo (void)
+{
+ _Complex float x;
+ __asm ("" : "=x" (x)); /* { dg-error "inconsistent register constraints" } */
+ return x;
+}
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] PR inline-asm/55934
2013-01-17 23:45 [patch] PR inline-asm/55934 Steven Bosscher
@ 2013-01-18 0:00 ` Jakub Jelinek
2013-01-18 19:32 ` Steven Bosscher
2013-01-19 0:15 ` Vladimir Makarov
1 sibling, 1 reply; 9+ messages in thread
From: Jakub Jelinek @ 2013-01-18 0:00 UTC (permalink / raw)
To: Steven Bosscher; +Cc: Vladimir Makarov, GCC Patches
On Fri, Jan 18, 2013 at 12:45:06AM +0100, Steven Bosscher wrote:
> --- testsuite/gcc.target/i386/pr55934.c (revision 0)
> +++ testsuite/gcc.target/i386/pr55934.c (revision 0)
> @@ -0,0 +1,10 @@
> +/* PR inline-asm/55934 */
> +/* { dg-do compile } */
> +/* { dg-require-effective-target sse } */
Don't you need /* { dg-options "-msse" } */ ? dg-require-effective-target sse
itself doesn't add -msse option...
> +_Complex float
> +foo (void)
> +{
> + _Complex float x;
> + __asm ("" : "=x" (x)); /* { dg-error "inconsistent register constraints" } */
> + return x;
> +}
Jakub
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] PR inline-asm/55934
2013-01-18 0:00 ` Jakub Jelinek
@ 2013-01-18 19:32 ` Steven Bosscher
0 siblings, 0 replies; 9+ messages in thread
From: Steven Bosscher @ 2013-01-18 19:32 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Vladimir Makarov, GCC Patches
On Fri, Jan 18, 2013 at 12:59 AM, Jakub Jelinek wrote:
> On Fri, Jan 18, 2013 at 12:45:06AM +0100, Steven Bosscher wrote:
>> --- testsuite/gcc.target/i386/pr55934.c (revision 0)
>> +++ testsuite/gcc.target/i386/pr55934.c (revision 0)
>> @@ -0,0 +1,10 @@
>> +/* PR inline-asm/55934 */
>> +/* { dg-do compile } */
>> +/* { dg-require-effective-target sse } */
>
> Don't you need /* { dg-options "-msse" } */ ? dg-require-effective-target sse
> itself doesn't add -msse option...
Actually, I needed even more than that:
/* { dg-options "-std=c99 -msse" } */
But I added that line after creating the diff, and forgot to update
the test case in the patch :-)
Ciao!
Steven
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] PR inline-asm/55934
2013-01-17 23:45 [patch] PR inline-asm/55934 Steven Bosscher
2013-01-18 0:00 ` Jakub Jelinek
@ 2013-01-19 0:15 ` Vladimir Makarov
2013-01-19 16:57 ` Steven Bosscher
1 sibling, 1 reply; 9+ messages in thread
From: Vladimir Makarov @ 2013-01-19 0:15 UTC (permalink / raw)
To: Steven Bosscher; +Cc: GCC Patches, Jakub Jelinek
On 13-01-17 6:45 PM, Steven Bosscher wrote:
> Hello Vlad,
>
> Attached is my attempt to fix PR55934, an error recovery issue in LRA
> with incorrect constraints in an asm.
>
> I'm not 100% sure this is all correct (especially the LRA insn data
> invalidating in lra-assigns.c) but it appears to fix the PR without
> introducing test suite failures.
The code is correct but call lra_invalidate_insn_data is not necessary
as the same thing will be done in lra_update_insn_recog_data (if what
lra_invalidate_insn_data does is not done yet) . So adding the
additional call is harmless as the result will be the same.
> Can you please review the patch?
The patch is ok for me except I'd prefer not to call
lra_invalidate_insn_data right before lra_update_insn_recog_data.
Thanks for working on the PR, Steven.
>
> gcc/
> PR inline-asm/55934
> * lra-assigns.c (assign_by_spills): Throw away the pattern of asms
> that have operands with impossible constraints.
> Add a FIXME for a speed-up opportunity.
> * lra-constraints.c (process_alt_operands): Verify that a class
> selected from constraints on asms is valid for the operand mode.
> (curr_insn_transform): Remove incorrect comment.
>
> testsuite/
> PR inline-asm/55934
> * gcc.target/i386/pr55934.c: New test.
>
> Index: lra-assigns.c
> ===================================================================
> --- lra-assigns.c (revision 195104)
> +++ lra-assigns.c (working copy)
> @@ -1240,6 +1240,24 @@ assign_by_spills (void)
> asm_p = true;
> error_for_asm (insn,
> "%<asm%> operand has impossible constraints");
> + /* Avoid further trouble with this insn.
> + For asm goto, instead of fixing up all the edges
> + just clear the template and clear input operands
> + (asm goto doesn't have any output operands). */
> + if (JUMP_P (insn))
> + {
> + rtx asm_op = extract_asm_operands (PATTERN (insn));
> + ASM_OPERANDS_TEMPLATE (asm_op) = ggc_strdup ("");
> + ASM_OPERANDS_INPUT_VEC (asm_op) = rtvec_alloc (0);
> + ASM_OPERANDS_INPUT_CONSTRAINT_VEC (asm_op) =
> rtvec_alloc (0);
> + lra_invalidate_insn_data (insn);
> + lra_update_insn_recog_data (insn);
> + }
> + else
> + {
> + PATTERN (insn) = gen_rtx_USE (VOIDmode, const0_rtx);
> + lra_set_insn_deleted (insn);
> + }
> }
> }
> lra_assert (asm_p);
> @@ -1263,6 +1281,9 @@ assign_by_spills (void)
> bitmap_ior_into (&changed_insns,
> &lra_reg_info[sorted_pseudos[i]].insn_bitmap);
> }
> +
> + /* FIXME: Look up the changed insns in the cached LRA insn data using
> + an EXECUTE_IF_SET_IN_BITMAP over changed_insns. */
> FOR_EACH_BB (bb)
> FOR_BB_INSNS (bb, insn)
> if (bitmap_bit_p (&changed_insns, INSN_UID (insn)))
> Index: lra-constraints.c
> ===================================================================
> --- lra-constraints.c (revision 195104)
> +++ lra-constraints.c (working copy)
> @@ -1847,11 +1847,27 @@ process_alt_operands (int only_alternati
> int const_to_mem = 0;
> bool no_regs_p;
>
> + /* If this alternative asks for a specific reg class, see if there
> + is at least one allocatable register in that class. */
> no_regs_p
> = (this_alternative == NO_REGS
> || (hard_reg_set_subset_p
> (reg_class_contents[this_alternative],
> lra_no_alloc_regs)));
> +
> + /* For asms, verify that the class for this alternative
> is possible
> + for the mode that is specified. */
> + if (!no_regs_p && REG_P (op) && INSN_CODE (curr_insn) < 0)
> + {
> + int i;
> + for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
> + if (HARD_REGNO_MODE_OK (i, mode)
> + && in_hard_reg_set_p
> (reg_class_contents[this_alternative], mode, i))
> + break;
> + if (i == FIRST_PSEUDO_REGISTER)
> + winreg = false;
> + }
> +
> /* If this operand accepts a register, and if the
> register class has at least one allocatable register,
> then this operand can be reloaded. */
> @@ -2742,10 +2758,6 @@ curr_insn_transform (void)
> swap_operands (commutative);
> }
>
> - /* The operands don't meet the constraints. goal_alt describes the
> - alternative that we could reach by reloading the fewest operands.
> - Reload so as to fit it. */
> -
> if (! alt_p && ! sec_mem_p)
> {
> /* No alternative works with reloads?? */
> Index: testsuite/gcc.target/i386/pr55934.c
> ===================================================================
> --- testsuite/gcc.target/i386/pr55934.c (revision 0)
> +++ testsuite/gcc.target/i386/pr55934.c (revision 0)
> @@ -0,0 +1,10 @@
> +/* PR inline-asm/55934 */
> +/* { dg-do compile } */
> +/* { dg-require-effective-target sse } */
> +_Complex float
> +foo (void)
> +{
> + _Complex float x;
> + __asm ("" : "=x" (x)); /* { dg-error "inconsistent register constraints" } */
> + return x;
> +}
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] PR inline-asm/55934
2013-01-19 0:15 ` Vladimir Makarov
@ 2013-01-19 16:57 ` Steven Bosscher
2013-01-21 16:23 ` Vladimir Makarov
0 siblings, 1 reply; 9+ messages in thread
From: Steven Bosscher @ 2013-01-19 16:57 UTC (permalink / raw)
To: Vladimir Makarov; +Cc: GCC Patches, Jakub Jelinek
On Sat, Jan 19, 2013 at 1:15 AM, Vladimir Makarov wrote:
> On 13-01-17 6:45 PM, Steven Bosscher wrote:
>>
>> Hello Vlad,
>>
>> Attached is my attempt to fix PR55934, an error recovery issue in LRA
>> with incorrect constraints in an asm.
>>
>> I'm not 100% sure this is all correct (especially the LRA insn data
>> invalidating in lra-assigns.c) but it appears to fix the PR without
>> introducing test suite failures.
>
> The code is correct but call lra_invalidate_insn_data is not necessary as
> the same thing will be done in lra_update_insn_recog_data (if what
> lra_invalidate_insn_data does is not done yet) .
That is what I expected, too. My first attempts didn't have the
lra_invalidate_insn_data call.
But I think lra_update_insn_recog_data calls lra_invalidate_insn_data
for asms. lra_invalidate_insn_data is called if there is existing
recog data but the insn code has changed:
if ((data = lra_insn_recog_data[uid]) != NULL
&& data->icode != INSN_CODE (insn))
{
invalidate_insn_data_regno_info (data, insn, get_insn_freq (insn));
invalidate_insn_recog_data (uid);
data = NULL;
}
For an asm, INSN_CODE==-1, so "data->icode != INSN_CODE (insn)" is
always false, and lra_invalidate_insn_data is never called. The result
is an ICE:
pr55512-3.c:15:1: internal compiler error: in
lra_update_insn_recog_data, at lra.c:1263
lra.c:1263 lra_assert (nop == data->insn_static_data->n_operands);
> So adding the additional
> call is harmless as the result will be the same.
Given my explanation above, do you think we should make
lra_update_insn_recog_data handle asms as a special case? E.g.:
Index: lra.c
===================================================================
--- lra.c (revision 195104)
+++ lra.c (working copy)
@@ -1239,7 +1239,8 @@ lra_update_insn_recog_data (rtx insn)
check_and_expand_insn_recog_data (uid);
if ((data = lra_insn_recog_data[uid]) != NULL
- && data->icode != INSN_CODE (insn))
+ && (data->icode != INSN_CODE (insn)
+ || asm_noperands (PATTERN (insn)) >= 0))
{
invalidate_insn_data_regno_info (data, insn, get_insn_freq (insn));
invalidate_insn_recog_data (uid);
Or just keep the lra_invalidate_insn_data call?
Ciao!
Steven
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] PR inline-asm/55934
2013-01-19 16:57 ` Steven Bosscher
@ 2013-01-21 16:23 ` Vladimir Makarov
2013-01-22 9:42 ` Steven Bosscher
0 siblings, 1 reply; 9+ messages in thread
From: Vladimir Makarov @ 2013-01-21 16:23 UTC (permalink / raw)
To: Steven Bosscher; +Cc: GCC Patches, Jakub Jelinek
On 13-01-19 11:57 AM, Steven Bosscher wrote:
> On Sat, Jan 19, 2013 at 1:15 AM, Vladimir Makarov wrote:
>> On 13-01-17 6:45 PM, Steven Bosscher wrote:
>>> Hello Vlad,
>>>
>>> Attached is my attempt to fix PR55934, an error recovery issue in LRA
>>> with incorrect constraints in an asm.
>>>
>>> I'm not 100% sure this is all correct (especially the LRA insn data
>>> invalidating in lra-assigns.c) but it appears to fix the PR without
>>> introducing test suite failures.
>> The code is correct but call lra_invalidate_insn_data is not necessary as
>> the same thing will be done in lra_update_insn_recog_data (if what
>> lra_invalidate_insn_data does is not done yet) .
> That is what I expected, too. My first attempts didn't have the
> lra_invalidate_insn_data call.
>
> But I think lra_update_insn_recog_data calls lra_invalidate_insn_data
> for asms. lra_invalidate_insn_data is called if there is existing
> recog data but the insn code has changed:
>
> if ((data = lra_insn_recog_data[uid]) != NULL
> && data->icode != INSN_CODE (insn))
> {
> invalidate_insn_data_regno_info (data, insn, get_insn_freq (insn));
> invalidate_insn_recog_data (uid);
> data = NULL;
> }
>
> For an asm, INSN_CODE==-1, so "data->icode != INSN_CODE (insn)" is
> always false, and lra_invalidate_insn_data is never called. The result
> is an ICE:
>
> pr55512-3.c:15:1: internal compiler error: in
> lra_update_insn_recog_data, at lra.c:1263
> lra.c:1263 lra_assert (nop == data->insn_static_data->n_operands);
>
>
>> So adding the additional
>> call is harmless as the result will be the same.
> Given my explanation above, do you think we should make
> lra_update_insn_recog_data handle asms as a special case? E.g.:
>
> Index: lra.c
> ===================================================================
> --- lra.c (revision 195104)
> +++ lra.c (working copy)
> @@ -1239,7 +1239,8 @@ lra_update_insn_recog_data (rtx insn)
>
> check_and_expand_insn_recog_data (uid);
> if ((data = lra_insn_recog_data[uid]) != NULL
> - && data->icode != INSN_CODE (insn))
> + && (data->icode != INSN_CODE (insn)
> + || asm_noperands (PATTERN (insn)) >= 0))
> {
> invalidate_insn_data_regno_info (data, insn, get_insn_freq (insn));
> invalidate_insn_recog_data (uid);
>
>
> Or just keep the lra_invalidate_insn_data call?
>
Yes, I guess you are right -- I missed a special treatment of asm in
lra_update_insn_recog_data. I'd prefer the above change than just
keeping lra_invalidate_insn_data call. I think it is more safe solution
for other parts of LRA code.
Thanks, Steven.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] PR inline-asm/55934
2013-01-21 16:23 ` Vladimir Makarov
@ 2013-01-22 9:42 ` Steven Bosscher
2013-01-23 18:43 ` Vladimir Makarov
0 siblings, 1 reply; 9+ messages in thread
From: Steven Bosscher @ 2013-01-22 9:42 UTC (permalink / raw)
To: Vladimir Makarov; +Cc: GCC Patches, Jakub Jelinek
On Mon, Jan 21, 2013 at 5:22 PM, Vladimir Makarov wrote:
> I'd prefer the above change than just keeping
> lra_invalidate_insn_data call. I think it is more safe solution for other
> parts of LRA code.
I agree, but unfortunately the compiler does not...
With that lra.c change, I get extra fails:
+FAIL: gcc.target/i386/20011029-2.c (internal compiler error)
+FAIL: gcc.target/i386/20011029-2.c (test for excess errors)
+FAIL: gcc.target/i386/pr21291.c (internal compiler error)
+FAIL: gcc.target/i386/pr21291.c (test for excess errors)
These are constrain_operands(1) failures in check_rtl. Apparently some
relevant info is lost in lra_update_insn_recog_data. Not sure how to
debug this...
Ciao!
Steven
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] PR inline-asm/55934
2013-01-22 9:42 ` Steven Bosscher
@ 2013-01-23 18:43 ` Vladimir Makarov
2013-01-24 10:31 ` Steven Bosscher
0 siblings, 1 reply; 9+ messages in thread
From: Vladimir Makarov @ 2013-01-23 18:43 UTC (permalink / raw)
To: Steven Bosscher; +Cc: GCC Patches, Jakub Jelinek
On 13-01-22 4:41 AM, Steven Bosscher wrote:
> On Mon, Jan 21, 2013 at 5:22 PM, Vladimir Makarov wrote:
>> I'd prefer the above change than just keeping
>> lra_invalidate_insn_data call. I think it is more safe solution for other
>> parts of LRA code.
> I agree, but unfortunately the compiler does not...
>
> With that lra.c change, I get extra fails:
>
> +FAIL: gcc.target/i386/20011029-2.c (internal compiler error)
> +FAIL: gcc.target/i386/20011029-2.c (test for excess errors)
> +FAIL: gcc.target/i386/pr21291.c (internal compiler error)
> +FAIL: gcc.target/i386/pr21291.c (test for excess errors)
>
> These are constrain_operands(1) failures in check_rtl. Apparently some
> relevant info is lost in lra_update_insn_recog_data. Not sure how to
> debug this...
>
Steven, sorry for the delay with the answer. I was busy with other things.
The error occurs because a pseudo in asm insn is not changed into hard
register as the pseudo info is incorrect after info updating.
You should just use lra_update_insn_regno_info. The patch (and the patch
is ok to commit) should look like
Index: lra-assigns.c
===================================================================
--- lra-assigns.c (revision 195278)
+++ lra-assigns.c (working copy)
@@ -1240,6 +1240,23 @@ assign_by_spills (void)
asm_p = true;
error_for_asm (insn,
"%<asm%> operand has impossible
constraints");
+ /* Avoid further trouble with this insn.
+ For asm goto, instead of fixing up all the edges
+ just clear the template and clear input operands
+ (asm goto doesn't have any output operands). */
+ if (JUMP_P (insn))
+ {
+ rtx asm_op = extract_asm_operands (PATTERN (insn));
+ ASM_OPERANDS_TEMPLATE (asm_op) = ggc_strdup ("");
+ ASM_OPERANDS_INPUT_VEC (asm_op) = rtvec_alloc (0);
+ ASM_OPERANDS_INPUT_CONSTRAINT_VEC (asm_op) =
rtvec_alloc (0);
+ lra_update_insn_regno_info (insn);
+ }
+ else
+ {
+ PATTERN (insn) = gen_rtx_USE (VOIDmode, const0_rtx);
+ lra_set_insn_deleted (insn);
+ }
}
}
lra_assert (asm_p);
@@ -1263,6 +1280,9 @@ assign_by_spills (void)
bitmap_ior_into (&changed_insns,
&lra_reg_info[sorted_pseudos[i]].insn_bitmap);
}
+
+ /* FIXME: Look up the changed insns in the cached LRA insn data using
+ an EXECUTE_IF_SET_IN_BITMAP over changed_insns. */
FOR_EACH_BB (bb)
FOR_BB_INSNS (bb, insn)
if (bitmap_bit_p (&changed_insns, INSN_UID (insn)))
Index: lra-constraints.c
===================================================================
--- lra-constraints.c (revision 195322)
+++ lra-constraints.c (working copy)
@@ -1847,11 +1847,27 @@ process_alt_operands (int only_alternati
int const_to_mem = 0;
bool no_regs_p;
+ /* If this alternative asks for a specific reg class, see
if there
+ is at least one allocatable register in that class. */
no_regs_p
= (this_alternative == NO_REGS
|| (hard_reg_set_subset_p
(reg_class_contents[this_alternative],
lra_no_alloc_regs)));
+
+ /* For asms, verify that the class for this alternative is
possible
+ for the mode that is specified. */
+ if (!no_regs_p && REG_P (op) && INSN_CODE (curr_insn) < 0)
+ {
+ int i;
+ for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
+ if (HARD_REGNO_MODE_OK (i, mode)
+ && in_hard_reg_set_p
(reg_class_contents[this_alternative], mode, i))
+ break;
+ if (i == FIRST_PSEUDO_REGISTER)
+ winreg = false;
+ }
+
/* If this operand accepts a register, and if the
register class has at least one allocatable register,
then this operand can be reloaded. */
@@ -2742,10 +2758,6 @@ curr_insn_transform (void)
swap_operands (commutative);
}
- /* The operands don't meet the constraints. goal_alt describes the
- alternative that we could reach by reloading the fewest operands.
- Reload so as to fit it. */
-
if (! alt_p && ! sec_mem_p)
{
/* No alternative works with reloads?? */
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [patch] PR inline-asm/55934
2013-01-23 18:43 ` Vladimir Makarov
@ 2013-01-24 10:31 ` Steven Bosscher
0 siblings, 0 replies; 9+ messages in thread
From: Steven Bosscher @ 2013-01-24 10:31 UTC (permalink / raw)
To: Vladimir Makarov; +Cc: GCC Patches, Jakub Jelinek
[-- Attachment #1: Type: text/plain, Size: 411 bytes --]
On Wed, Jan 23, 2013 at 7:43 PM, Vladimir Makarov wrote:
> The error occurs because a pseudo in asm insn is not changed into hard
> register as the pseudo info is incorrect after info updating.
>
> You should just use lra_update_insn_regno_info. The patch (and the patch is
> ok to commit) should look like
Right, that solves the problem. Thanks for the help!
I've committed the attached patch.
Ciao!
Steven
[-- Attachment #2: pr55934.diff --]
[-- Type: application/octet-stream, Size: 3899 bytes --]
gcc/
PR inline-asm/55934
* lra-assigns.c (assign_by_spills): Throw away the pattern of asms
that have operands with impossible constraints.
Add a FIXME for a speed-up opportunity.
* lra-constraints.c (process_alt_operands): Verify that a class
selected from constraints on asms is valid for the operand mode.
(curr_insn_transform): Remove incorrect comment.
testsuite/
PR inline-asm/55934
* gcc.target/i386/pr55934.c: New test.
Index: lra-assigns.c
===================================================================
--- lra-assigns.c (revision 195104)
+++ lra-assigns.c (working copy)
@@ -1240,6 +1240,23 @@ assign_by_spills (void)
asm_p = true;
error_for_asm (insn,
"%<asm%> operand has impossible constraints");
+ /* Avoid further trouble with this insn.
+ For asm goto, instead of fixing up all the edges
+ just clear the template and clear input operands
+ (asm goto doesn't have any output operands). */
+ if (JUMP_P (insn))
+ {
+ rtx asm_op = extract_asm_operands (PATTERN (insn));
+ ASM_OPERANDS_TEMPLATE (asm_op) = ggc_strdup ("");
+ ASM_OPERANDS_INPUT_VEC (asm_op) = rtvec_alloc (0);
+ ASM_OPERANDS_INPUT_CONSTRAINT_VEC (asm_op) = rtvec_alloc (0);
+ lra_update_insn_regno_info (insn);
+ }
+ else
+ {
+ PATTERN (insn) = gen_rtx_USE (VOIDmode, const0_rtx);
+ lra_set_insn_deleted (insn);
+ }
}
}
lra_assert (asm_p);
@@ -1263,6 +1280,9 @@ assign_by_spills (void)
bitmap_ior_into (&changed_insns,
&lra_reg_info[sorted_pseudos[i]].insn_bitmap);
}
+
+ /* FIXME: Look up the changed insns in the cached LRA insn data using
+ an EXECUTE_IF_SET_IN_BITMAP over changed_insns. */
FOR_EACH_BB (bb)
FOR_BB_INSNS (bb, insn)
if (bitmap_bit_p (&changed_insns, INSN_UID (insn)))
Index: lra-constraints.c
===================================================================
--- lra-constraints.c (revision 195104)
+++ lra-constraints.c (working copy)
@@ -1847,11 +1847,27 @@ process_alt_operands (int only_alternati
int const_to_mem = 0;
bool no_regs_p;
+ /* If this alternative asks for a specific reg class, see if there
+ is at least one allocatable register in that class. */
no_regs_p
= (this_alternative == NO_REGS
|| (hard_reg_set_subset_p
(reg_class_contents[this_alternative],
lra_no_alloc_regs)));
+
+ /* For asms, verify that the class for this alternative is possible
+ for the mode that is specified. */
+ if (!no_regs_p && REG_P (op) && INSN_CODE (curr_insn) < 0)
+ {
+ int i;
+ for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
+ if (HARD_REGNO_MODE_OK (i, mode)
+ && in_hard_reg_set_p (reg_class_contents[this_alternative], mode, i))
+ break;
+ if (i == FIRST_PSEUDO_REGISTER)
+ winreg = false;
+ }
+
/* If this operand accepts a register, and if the
register class has at least one allocatable register,
then this operand can be reloaded. */
@@ -2742,10 +2758,6 @@ curr_insn_transform (void)
swap_operands (commutative);
}
- /* The operands don't meet the constraints. goal_alt describes the
- alternative that we could reach by reloading the fewest operands.
- Reload so as to fit it. */
-
if (! alt_p && ! sec_mem_p)
{
/* No alternative works with reloads?? */
Index: testsuite/gcc.target/i386/pr55934.c
===================================================================
--- testsuite/gcc.target/i386/pr55934.c (revision 0)
+++ testsuite/gcc.target/i386/pr55934.c (revision 0)
@@ -0,0 +1,11 @@
+/* PR inline-asm/55934 */
+/* { dg-do compile } */
+/* { dg-require-effective-target sse } */
+/* { dg-options "-std=c99 -msse" } */
+_Complex float
+foo (void)
+{
+ _Complex float x;
+ __asm ("" : "=x" (x)); /* { dg-error "inconsistent .* constraint" } */
+ return x;
+}
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-01-24 10:31 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-17 23:45 [patch] PR inline-asm/55934 Steven Bosscher
2013-01-18 0:00 ` Jakub Jelinek
2013-01-18 19:32 ` Steven Bosscher
2013-01-19 0:15 ` Vladimir Makarov
2013-01-19 16:57 ` Steven Bosscher
2013-01-21 16:23 ` Vladimir Makarov
2013-01-22 9:42 ` Steven Bosscher
2013-01-23 18:43 ` Vladimir Makarov
2013-01-24 10:31 ` Steven Bosscher
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).