[PATCH] Fix problem in the expansion of ROTATE expressions

[Olivier Hainque <hainque at act-europe dot fr>]

Hello,

The Ada testcase below is miscompiled by the current mainline compiler on many
targets, leading to Program_Error beeing raised while it should not.

     with Interfaces;  use Interfaces;

     procedure Rotate_64 is

	type Ptr_To_Unsigned_64 is access all Unsigned_64;

	Value : aliased Unsigned_64;
	Alias : Ptr_To_Unsigned_64 := Value'Access;

     begin
	Value := 16#0000_0000_1234_5678#;

	Alias.all := Rotate_Right (Value, Amount => 32);

	if Alias.all /= 16#1234_5678_0000_0000# then
	   raise Program_Error;
	end if;
     end;

For instance, on i686-pc-linux-gnu with 3.5.0 20040320 (experimental):

     $ gnatmake -f rotate_64.adb && ./rotate_64
     gcc -c rotate_64.adb
     gnatbind -x rotate_64.ali
     gnatlink rotate_64.ali

     raised PROGRAM_ERROR : rotate_64.adb:16 explicit raise

--

The problem comes from the code performing

  /* Synthesize double word rotates from single word shifts.  */

in expand_binop, where we get with op0 and target two diffent MEM RTXs
designating the same memory location:

     (gdb) pr op0
     (mem/f:DI (plus:SI (reg/f:SI 54 virtual-stack-vars)
	     (const_int -8 [0xfffffff8])) [0 value+0 S8 A64])

     (gdb) pr target
     (mem:DI (reg/f:SI 59) [0 S8 A64])

     (insn 9 40 10 0 (parallel [
            (set (reg/f:SI 58)
                (plus:SI (reg/f:SI 20 frame)
                    (const_int -8 [0xfffffff8])))


     (insn 10 9 12 0 (set (mem/u/f:SI (plus:SI (reg/f:SI 20 frame)
                          (const_int -12 [0xfffffff4])) [0 alias+0 S4 A32])
                      (reg/f:SI 58))

     (insn 14 12 15 0 (set (reg/f:SI 59 [ alias ])
        (mem/u/f:SI (plus:SI (reg/f:SI 20 frame)
                (const_int -12 [0xfffffff4])) [0 alias+0 S4 A32]))

The current test deciding whether a new (register) target should be used does
not trigger, and for the testcase above we end up in:

      if (shift_count == BITS_PER_WORD)
	{
	  /* This is just a word swap.  */
	  emit_move_insn (outof_target, into_input);
	  emit_move_insn (into_target, outof_input);

The idea is to issue something like
 
      o-------------------o
      |    o---------o    |
      v    v         |    |
    |----|----|    |----|----|
       target          op0

which is wrong when target and op0 are MEMs designating the same location
because it becomes

     o-----o
     v     |
   |----|----|  target/op0
     |     ^
     o-----o

which clobbers part of the input too early. Although it is less obvious from
the code, at least 16bit rotates also have it wrong.

--

The patch below fixes that by forcing the use of a new target (register) as
soon as target is a MEM, which seems a good thing in general anyway.

Bootstrap and regression testing successful for all languages on
i686-pc-linux-gnu.

Ok ?

Thanks in advance,

Olivier

--

2004-03-23  Olivier Hainque  <hainque@act-europe.fr>

	* optabs.c (expand_binop): When synthesizing double word rotates from
	single word shifts, use a new target if the provided one is a MEM.

*** gcc/optabs.c.ori	Fri Mar 19 16:50:47 2004
--- gcc/optabs.c	Fri Mar 19 17:03:32 2004
*************** expand_binop (mode, binoptab, op0, op1, 
*** 1070,1077 ****
        int shift_count, left_shift, outof_word;
  
        /* If TARGET is the same as one of the operands, the REG_EQUAL note
! 	 won't be accurate, so use a new target.  */
!       if (target == 0 || target == op0 || target == op1)
  	target = gen_reg_rtx (mode);
  
        start_sequence ();
--- 1070,1082 ----
        int shift_count, left_shift, outof_word;
  
        /* If TARGET is the same as one of the operands, the REG_EQUAL note
! 	 won't be accurate, so use a new target. Do this also if target is a
! 	 mem, first because having a register instead opens the way to later
! 	 optimization oportunities, and second because if op0 happens to be
! 	 a mem designating the same location, we would risk clobbering it
! 	 too early in the code sequence we generate below.  */
!       if (target == 0 || target == op0 || target == op1
! 	  || GET_CODE (target) == MEM)
  	target = gen_reg_rtx (mode);
  
        start_sequence ();