This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[IA-64 PATCH] PR rtl-opt/14261: ifcvt.c vs emit_move_insn.


The following patch is my proposed solution to PR rtl-optimization/14261
which is an unrecognizable insn ICE compiling the attached testcase on
IA-64.  The root of the problem is a "miscommunication" between ifcvt.c
and the IA-64 backend.

As previously analyzed by Jim Wilson, the middle-end should never emit
instructions without recognizing them.  However, in this case, and
probably as the result of changes to ifcvt.c:noce_emit_move_insn since
Jim's original analysis, it's not the middle-end that's emitting the
invalid instruction but the IA-64 backend!  Whilst its true that insn
patterns constructed by the middle-end need to be recognized, insns
emitted by a backend's define_expand interface are assumed to always
be valid, or corrected by later splitters/peepholes.

The suspicious chain of events is that the middle-end calls the
function emit_move_insn with a dst of (zero_extract (reg) 0 32)
and a pseudo register source.  Emit emit_move_insn_1, then in
turn calls ia64.c's "movdi" expander, the destination matches its
general_operand constraint, the invalid insn is emitted, and then
fails to match the ia64.md's *movdi_internal define_insn, which
requires the destination to be a "destination_operand".
Interestingly, ia64_expand_move never inspects or adjusts the
destination RTX (except for pointer assignments).

The next question is why does the middle-end believe that it can
generate a move to (zero_extract (reg) 0 32) on IA-64.  The answer
is that the middle-end didn't dream up this RTX, but instead
already found it in the RTL stream, as the SET_DEST of a single_set
insn.  The IA-64 "insnv" expander generates them, with suitable
clobbers.

The middle-end is cautious about manipulating any SET_DEST that
isn't a simple register, so ifcvt.c will only call the expansion
machinery to handle them, and only then if no_new_pseudos allows
the allocation of new temporaries.

The "miscommunication" then, is what is the middle-end allowed to
pass emit_move_insn as a source or destination.  Unfortunately,
this is poorly defined, even though the middle-end assumes that
calls to mov[sd]i with a word_mode operand can never fail.  There
is some wording that the backend has to generate RTL for moves
when either source or destination is a register, to allow reload
to do its job.  Hence tightening ia64.md's movdi constraints
wouldn't be a fix.

The solution proposed below, suggests the unwritten rule that if
a backend generates identifable single_set insns, then either
the source or destination should be handled by the corresponding
mov[sd]i expander.  Perhaps not unreasonable, even though the fix
below looks like a hack/workaround.  I agree with Jim's previous
analysis that allowing emit_move_insn to fail opens up a whole
can of worms (in his words "looks like a bit of work" :).

The following patch has been tested ia64-unknown-linux-gnu, with
a full "make bootstrap", all default languages, and regression
tested with a top-level "make -k check" with no new failures.
My apologies for using the PR number as the testcase file name,
but I didn't want to classify the failure as purely ifcvt or ia64.

Ok for mainline?



2006-02-26  Roger Sayle  <roger@eyesopen.com>

	PR rtl-optimization/14261
	* config/ia64/ia64.md (movdi): If given a suitable zero_extract
	as the destination, call gen_insv to generate an insv insn.

	* testsuite/gcc.dg/pr14261-1.c: New test case.


Index: ia64.md
===================================================================
*** ia64.md	(revision 110686)
--- ia64.md	(working copy)
***************
*** 331,337 ****
  	(match_operand:DI 1 "general_operand" ""))]
    ""
  {
!   rtx op1 = ia64_expand_move (operands[0], operands[1]);
    if (!op1)
      DONE;
    operands[1] = op1;
--- 331,353 ----
  	(match_operand:DI 1 "general_operand" ""))]
    ""
  {
!   rtx op1;
!
!   /* If this looks like an insv insn, call gen_insv.  */
!   if (GET_CODE (operands[0]) == ZERO_EXTRACT
!       && GET_CODE (XEXP (operands[0], 0)) == REG
!       && GET_CODE (XEXP (operands[0], 1)) == CONST_INT
!       && GET_CODE (XEXP (operands[0], 2)) == CONST_INT
!       && GET_CODE (operands[1]) == REG)
!     {
!       emit_insn (gen_insv (XEXP (operands[0], 0),
! 			   XEXP (operands[0], 1),
! 			   XEXP (operands[0], 2),
! 			   operands[1]));
!       DONE;
!     }
!
!   op1 = ia64_expand_move (operands[0], operands[1]);
    if (!op1)
      DONE;
    operands[1] = op1;


/* PR rtl-optimization/14261  */
/* { dg-do compile } */
/* { dg-options "-O2" } */

typedef union YYSTYPE {
    double dval;
    int intval;
} YYSTYPE;

YYSTYPE *x;

void
knparse (void)
{
  YYSTYPE yyvsa[200];
  register YYSTYPE *yyvsp;
  YYSTYPE yyval, *x;
  yyvsp = &yyvsa[100];
  if (yyvsp[3].intval > yyvsp[0].intval)
     yyval.intval = yyvsp[3].intval;
  else
     yyval.intval = yyvsp[0].intval;
  *x = yyval;
}


Roger
--


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]