Re: [PATCH] ifcvt: Handle INSV patterns properly when a temp reg is needed

[Andreas Krebbel <krebbel1 at de dot ibm dot com>]

Hi Roger,

> At that point I gave up :-(.  If you're interested in tackling
> the BIS_BIG_ENDIAN issue, I'm happy to help by bootstrapping and
> regression testing candidate patches on ia64-unknown-linux-gnu
> and ia64-hp-hpux11.2x if you don't have access to them.
Thanks for the offer. What do you think about the attached patch?
With the patch noce_emit_move_insn undoes the adjustment done by 
store_bit_field for BITS_BIG_ENDIAN != BYTES_BIG_ENDIAN machines 
before calling store_bit_field again.
Could you please test it if you think it looks promising?

> I'd also prefer to hold off on approving your "paradoxical SUBREGs
> in store_bit_field" patch, until we've confirmed that that change
> is needed.  For 4.1, we may decide that its safer just to prevent
> any ZERO_EXTRACTs from reaching this part of ifcvt.
For S/390 the paradoxical subreg changes are also needed with
this patch.

I agree that the changes might be to dangerous for gcc 4.1. What do
think about the trivial fix I've proposed in my last mail?
http://gcc.gnu.org/ml/gcc-patches/2006-03/msg01811.html

Bye,

-Andreas-

Index: gcc/ifcvt.c
===================================================================
*** gcc/ifcvt.c.orig	2006-04-03 10:53:01.000000000 +0200
--- gcc/ifcvt.c	2006-04-03 13:24:06.000000000 +0200
*************** noce_emit_move_insn (rtx x, rtx y)
*** 702,748 ****
        end_sequence();
  
        if (recog_memoized (insn) <= 0)
! 	switch (GET_RTX_CLASS (GET_CODE (y)))
! 	  {
! 	  case RTX_UNARY:
! 	    ot = code_to_optab[GET_CODE (y)];
! 	    if (ot)
! 	      {
! 		start_sequence ();
! 		target = expand_unop (GET_MODE (y), ot, XEXP (y, 0), x, 0);
! 		if (target != NULL_RTX)
! 		  {
! 		    if (target != x)
! 		      emit_move_insn (x, target);
! 		    seq = get_insns ();
! 		  }
! 		end_sequence ();
! 	      }
! 	    break;
! 
! 	  case RTX_BIN_ARITH:
! 	  case RTX_COMM_ARITH:
! 	    ot = code_to_optab[GET_CODE (y)];
! 	    if (ot)
! 	      {
! 		start_sequence ();
! 		target = expand_binop (GET_MODE (y), ot,
! 				       XEXP (y, 0), XEXP (y, 1),
! 				       x, 0, OPTAB_DIRECT);
! 		if (target != NULL_RTX)
! 		  {
! 		    if (target != x)
! 		      emit_move_insn (x, target);
! 		    seq = get_insns ();
! 		  }
! 		end_sequence ();
! 	      }
! 	    break;
  
! 	  default:
! 	    break;
! 	  }
  
        emit_insn (seq);
        return;
      }
--- 702,777 ----
        end_sequence();
  
        if (recog_memoized (insn) <= 0)
! 	{
! 	  if (GET_CODE (x) == ZERO_EXTRACT)
! 	    {
! 	      rtx op = XEXP (x, 0);
! 	      unsigned HOST_WIDE_INT size = INTVAL (XEXP (x, 1));
! 	      unsigned HOST_WIDE_INT start = INTVAL (XEXP (x, 2));
! 
! 	      /* store_bit_field expects START to be relative to 
! 		 BYTES_BIG_ENDIAN and adjusts this value for machines with 
! 		 BITS_BIG_ENDIAN != BYTES_BIG_ENDIAN.  In order to be able to 
! 		 invoke store_bit_field again it is necessary to have the START
! 		 value from the first call.  */
! 	      if (BITS_BIG_ENDIAN != BYTES_BIG_ENDIAN)
! 		{
! 		  if (MEM_P (op))
! 		    start = BITS_PER_UNIT - start - size;
! 		  else
! 		    {
! 		      gcc_assert (REG_P (op));
! 		      start = BITS_PER_WORD - start - size;
! 		    }
! 		}
  
! 	      gcc_assert (start < (MEM_P (op) ? BITS_PER_UNIT : BITS_PER_WORD));
! 	      store_bit_field (op, size, start, GET_MODE (x), y);
! 	      return;
! 	    }
  
+ 	  switch (GET_RTX_CLASS (GET_CODE (y)))
+ 	    {
+ 	    case RTX_UNARY:
+ 	      ot = code_to_optab[GET_CODE (y)];
+ 	      if (ot)
+ 		{
+ 		  start_sequence ();
+ 		  target = expand_unop (GET_MODE (y), ot, XEXP (y, 0), x, 0);
+ 		  if (target != NULL_RTX)
+ 		    {
+ 		      if (target != x)
+ 			emit_move_insn (x, target);
+ 		      seq = get_insns ();
+ 		    }
+ 		  end_sequence ();
+ 		}
+ 	      break;
+ 	      
+ 	    case RTX_BIN_ARITH:
+ 	    case RTX_COMM_ARITH:
+ 	      ot = code_to_optab[GET_CODE (y)];
+ 	      if (ot)
+ 		{
+ 		  start_sequence ();
+ 		  target = expand_binop (GET_MODE (y), ot,
+ 					 XEXP (y, 0), XEXP (y, 1),
+ 					 x, 0, OPTAB_DIRECT);
+ 		  if (target != NULL_RTX)
+ 		    {
+ 		      if (target != x)
+ 			  emit_move_insn (x, target);
+ 		      seq = get_insns ();
+ 		    }
+ 		  end_sequence ();
+ 		}
+ 	      break;
+ 	      
+ 	    default:
+ 	      break;
+ 	    }
+ 	}
+       
        emit_insn (seq);
        return;
      }
*************** noce_process_if_block (struct ce_if_bloc
*** 2231,2236 ****
--- 2260,2269 ----
      {
        if (no_new_pseudos || GET_MODE (x) == BLKmode)
  	return FALSE;
+ 
+       if (GET_MODE (x) == ZERO_EXTRACT && GET_CODE (XEXP (x, 2)) != CONST_INT)
+ 	return FALSE;
+ 	  
        x = gen_reg_rtx (GET_MODE (GET_CODE (x) == STRICT_LOW_PART
  				 ? XEXP (x, 0) : x));
      }
Index: gcc/testsuite/gcc.c-torture/compile/20060331-1.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.c-torture/compile/20060331-1.c	2006-04-03 10:53:14.000000000 +0200
***************
*** 0 ****
--- 1,21 ----
+ /* This failed because if conversion didn't handle insv patterns properly.  */
+ 
+ union y
+ {
+   int a;
+   unsigned short b;
+ };
+ 
+ extern void bar (unsigned short u, union y v);
+ 
+ void
+ foo (int check)
+ {
+   union y x;
+ 
+   if (check != 0)
+     x.b = 1;
+   else
+     x.b = 2;
+   bar (x.b, x);
+ }


> 

> 
> 
> Roger
> --