From: Vladimir Makarov <vmakarov@redhat.com>
To: Steven Bosscher <stevenb.gcc@gmail.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>, Jakub Jelinek <jakub@redhat.com>
Subject: Re: [patch] PR inline-asm/55934
Date: Sat, 19 Jan 2013 00:15:00 -0000 [thread overview]
Message-ID: <50F9E596.7000100@redhat.com> (raw)
In-Reply-To: <CABu31nOXZ7=j11mwxFWUXotLL7+XqaGM4TK0v9YRQhZ0k7Q45A@mail.gmail.com>
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;
> +}
next prev parent reply other threads:[~2013-01-19 0:15 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-01-17 23:45 Steven Bosscher
2013-01-18 0:00 ` Jakub Jelinek
2013-01-18 19:32 ` Steven Bosscher
2013-01-19 0:15 ` Vladimir Makarov [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50F9E596.7000100@redhat.com \
--to=vmakarov@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
--cc=stevenb.gcc@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).