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]

[Committed] PR27861: Mode mismatch RTL expanding rotates


The following patch resolves PR target/27861 which is P1 ICE-on-valid
regression affecting all 4.x releases on targets with promoted argument
modes, SHIFT_COUNT_TRUNCATED and no rotate insn expanders.

The problem is that RTL expansion plays fairly fast and loose with the
mode of the shift count in shift and rotate instructions, stripping
SUBREGs on SHIFT_COUNT_TRUNCATED targets to allow shift and rotate
expanders/patterns on some targets (such as the alpha) to see a raw
register.  Unfortunately, this can cause problems on machines without
rotate expanders, when after failing to find an optab we attempt to
synthesize a rotate using (x << y) | (x >> (C - y)).  To do this we
use make_tree to construct a VAR_DECL tree with a specified DECL_RTL.
Unfortunately, due to SUBREG stripping we may end up with a temporary
tree node who's DECL_RTL has a different mode from the node's type.
This then causes an ICE in gcc's sanity checking.

The solution is to add back any stripped SUBREG, by explicitly calling
convert_to_mode if the modes don't match, to produce an RTL expression
of the correct mode before calling make_tree.  It's better to add a
new SUBREG rather than use the original "op1" as we may have updated
it's contents based on SHIFT_COUNT semantics.

Whilst there I also noticed another potential problem, that in the
construction of (x << y) | (x >> (C - y)), we were expanding the
tree for "y" twice, which is potentially unsafe if y has side-effects.
Normally, this is latent in C/C++-like languages, as we only recognize
rotates from the above idiom if the operands don't have side-effects.
However, its a simple enough fix to use "new_amount" instead of
"amount" and re-use the RTL result for both "y".


The following patch has been tested with a cc1 cross-compiler to
mipsel-linux-gnu, where it resolves the ICE in the testcase below,
and on x86_64-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.

Committed to mainline as revision 114773.  I'll backport to the
release branches after a while without problems.



2006-06-19  Roger Sayle  <roger@eyesopen.com>

	PR target/27861
	* expmed.c (expand_shift): On SHIFT_COUNT_TRUNCATED targets, we may
	have stripped a SUBREG from the shift count, so we may need to
	convert_to_mode back to the type's mode before calling make_tree.
	Use new_amount instead of amount to avoid expanding a tree twice.

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


Index: expmed.c
===================================================================
*** expmed.c	(revision 114700)
--- expmed.c	(working copy)
*************** expand_shift (enum tree_code code, enum
*** 2260,2272 ****
  		 code below.  */

  	      rtx subtarget = target == shifted ? 0 : target;
  	      rtx temp1;
  	      tree type = TREE_TYPE (amount);
! 	      tree new_amount = make_tree (type, op1);
! 	      tree other_amount
  		= fold_build2 (MINUS_EXPR, type,
  			       build_int_cst (type, GET_MODE_BITSIZE (mode)),
! 			       amount);

  	      shifted = force_reg (mode, shifted);

--- 2260,2276 ----
  		 code below.  */

  	      rtx subtarget = target == shifted ? 0 : target;
+ 	      tree new_amount, other_amount;
  	      rtx temp1;
  	      tree type = TREE_TYPE (amount);
! 	      if (GET_MODE (op1) != TYPE_MODE (type)
! 		  && GET_MODE (op1) != VOIDmode)
! 		op1 = convert_to_mode (TYPE_MODE (type), op1, 1);
! 	      new_amount = make_tree (type, op1);
! 	      other_amount
  		= fold_build2 (MINUS_EXPR, type,
  			       build_int_cst (type, GET_MODE_BITSIZE (mode)),
! 			       new_amount);

  	      shifted = force_reg (mode, shifted);


/* PR target/27861 */
/* The following code used to cause an ICE during RTL expansion, as
   expand shift was stripping the SUBREG of a rotate shift count, and
   later producing a VAR_DECL tree whose DECL_RTL's mode didn't match
   the VAR_DECL's type's mode.  */
/* { dg-do compile } */
/* { dg-options "-O2" } */

typedef struct sim_state *SIM_DESC;
typedef enum
{
  SIM_OPEN_STANDALONE, SIM_OPEN_DEBUG
}
SIM_RC;
typedef unsigned int unsigned32 __attribute__ ((__mode__ (__SI__)));
typedef unsigned int unsigned64 __attribute__ ((__mode__ (__DI__)));
typedef unsigned32 unsigned_address;
typedef unsigned_address address_word;
static __inline__ unsigned64
  __attribute__ ((__unused__)) ROTR64 (unsigned64 val, int shift)
{
  unsigned64 result;
  result = (((val) >> (shift)) | ((val) << ((64) - (shift))));
  return result;
}
typedef struct _sim_cpu sim_cpu;
enum
{
    TRACE_MEMORY_IDX, TRACE_MODEL_IDX, TRACE_ALU_IDX, TRACE_CORE_IDX,
};
typedef struct _trace_data
{
  char trace_flags[32];
}
TRACE_DATA;
typedef enum
{
    nr_watchpoint_types,
}
watchpoint_type;
typedef struct _sim_watchpoints
{
  TRACE_DATA trace_data;
}
sim_cpu_base;
struct _sim_cpu
{
  sim_cpu_base base;
};
struct sim_state
{
  sim_cpu cpu[1];
};
typedef address_word instruction_address;
do_dror (SIM_DESC sd, instruction_address cia, int MY_INDEX, unsigned64 x,
	 unsigned64 y)
{
  unsigned64 result;
  result = ROTR64 (x, y);
    {
      if ((((-1) & (1 << (TRACE_ALU_IDX))) != 0
	   && (((&(((&(sd)->cpu[0])))->base.trace_data))->
	       trace_flags)[TRACE_ALU_IDX] != 0))
	trace_result_word1 (sd, ((&(sd)->cpu[0])), TRACE_ALU_IDX, (result));
    }
}


Roger
--


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