gcc 4.6.1 expand is playing tricks on me

gcc 4.6.1 expand is playing tricks on me

[Paulo J. Matos]

Hi,

I got a few size regressions when moving from 4.5.3 to 4.6.1, all due to 
the same issue.

I have code that is basically a double word memory move:
void simple1(uint32 *a, uint32 *b) { *a = *b; }

GCC 4.6.1 is from this gimple:
simple1 (uint32 * a, uint32 * b)
{
   uint32 D.1927;

   # BLOCK 2 freq:10000
   # PRED: ENTRY [100.0%]  (fallthru,exec)
   D.1927_2 = *b_1(D);
   *a_3(D) = D.1927_2;
   return;
   # SUCC: EXIT [100.0%]

}

calling gen_movhi twice with:
call1 :
operand[0]: (reg:HI 19 [ D.1927 ])
operand[1]: (mem:HI (reg/v/f:QI 21 [ b ]))

call2 :
operand[0]: (mem:HI (reg/v/f:QI 20 [ a ]))
operand[1]: (reg:HI 19 [ D.1927 ])

while GCC 4.5.3 for _exactly_ the same gimple (as far as it is shown in 
the logs) it only calls gen_movhi with:
operand[0]: (mem:HI (reg/v/f:QI 20 [ a ]))
operand[1]: (mem:HI (reg/v/f:QI 21 [ b ]))

This seems to boil down to code that looks exactly the same between 
4.5.3-4.6.1 in cfgexpand.c, expand_gimple_basic_block:
	      def_operand_p def_p;
	      def_p = SINGLE_SSA_DEF_OPERAND (stmt, SSA_OP_DEF);

	      if (def_p != NULL)
		{
		  /* Ignore this stmt if it is in the list of
		     replaceable expressions.  */
		  if (SA.values
		      && bitmap_bit_p (SA.values,
				       SSA_NAME_VERSION (DEF_FROM_PTR (def_p))))
		    continue;
		}
	      last = expand_gimple_stmt (stmt);
	      maybe_dump_rtl_for_gimple_stmt (stmt, last);
	

gcc4.5.3 hits continue the first time it gets there and gcc4.6.1 fails 
the inner if and enters expand_gimple_stmt twice.

 From the comment in ssaexpand.h for the definition of SA.values, it says:
   /* For an SSA name version V bit V is set iff TER decided that 
 

      its definition should be forwarded.  */
   bitmap values;

What is TER? Any hints on why GCC 4.6.1 has now a different behaviour to 
GCC 4.5.3?

Cheers,
-- 
Paulo Matos

Re: gcc 4.6.1 expand is playing tricks on me

[Michael Matz]

Hi,

On Fri, 8 Jul 2011, Paulo J. Matos wrote:

> gcc4.5.3 hits continue the first time it gets there and gcc4.6.1 fails 
> the inner if and enters expand_gimple_stmt twice.

Yes, the MEMREF branch merge disabled TER (temporary expression 
replacement, tree-ssa-ter.c) for loads with stores that possibly alias, 
because expand doesn't deal correctly with all cases.

Without TERing these two instructions expand won't see both memory 
references at the same time, and hence generate separate load and store 
instruction, instead of a mem-mem move if that's supported on your target 
(I assume so, otherwise you wouldn't have noticed).

The question is, why doesn't combine merge the two separate load and store 
insns again into one?

If you feel adventurous you can try with the below patch.  Test it also 
with the following testcase:
---------------------------
char str[9] = "1234";

void
bar (void)
{
  unsigned int temp;
  char *p = &str[2];

  memcpy (&temp, &str[1], 4);
  memcpy (p, &temp, 4);
}
---------------------------

If expand still has the problem and you apply the patch, then on some 
targets this will emit wrong instructions because the two sides of the 
MEM-MEM assignment implicitely constructed and given to expand will 
partially overlap.


Ciao,
Michael.
-- 
Index: tree-ssa-ter.c
===================================================================
--- tree-ssa-ter.c      (revision 175921)
+++ tree-ssa-ter.c      (working copy)
@@ -638,6 +638,7 @@ find_replaceable_in_bb (temp_expr_table_
                 is a load aliasing it avoid creating overlapping
                 assignments which we cannot expand correctly.  */
              if (gimple_vdef (stmt)
+                 && 0
                  && gimple_assign_single_p (stmt))
                {
                  gimple def_stmt = SSA_NAME_DEF_STMT (use);

Re: gcc 4.6.1 expand is playing tricks on me

[Paulo J. Matos]

On 08/07/11 15:35, Michael Matz wrote:
> Without TERing these two instructions expand won't see both memory
> references at the same time, and hence generate separate load and store
> instruction, instead of a mem-mem move if that's supported on your target
> (I assume so, otherwise you wouldn't have noticed).
>
> The question is, why doesn't combine merge the two separate load and store
> insns again into one?
>

I got it! Thanks Michael.
The answer to your last question is that we don't support memory to 
memory moves.

The situation is a bit more complex. When we see
(set (mem:HI (reg:HI 0)) (mem:HI (reg:HI 1)))

we have a rule that expands this to 4 insn
(set (reg:QI AL) (mem:QI (plus (reg:HI 1) (const_int 1))))
(set (reg:QI AH) (mem:QI (reg:HI 1)))
(set (mem:QI (plus (reg:HI 0) (const_int 1))) (reg:QI AL))
(set (mem:QI (reg:HI 0)) (reg:QI AH))

because we know that using AL and AH as intermediate registers is the 
only way to go in this case. This is then modified slightly by combine, 
etc to generate optimal code.

When GCC 4.6.1 generates two insn dealing with HI so we never have the 
opportunity to introduce these set of 4 insn. The resulting code is less 
than perfect.

However, you hinted to using combine. I am wondering if I can combine 
and into a memory-memory move in HImode and straight away split into the 
4 insn above. In the end 4.6.1 would end up doing the same at combine 
time as 4.5.3 in expand time. I have to look at it once again.

As a last resort I might try your patch, however I try to touch the core 
as little as possible. We already have enough patches to deal with 16 
bit words, chars, wide chars, fn ptrs size != data ptrs size, etc. :)

Thank you very much for your explanation and tips.

Cheers,

Paulo Matos

Re: gcc 4.6.1 expand is playing tricks on me

[Paulo J. Matos]

On 08/07/11 15:58, Paulo J. Matos wrote:
> However, you hinted to using combine. I am wondering if I can combine
> and into a memory-memory move in HImode and straight away split into the
> 4 insn above. In the end 4.6.1 would end up doing the same at combine
> time as 4.5.3 in expand time. I have to look at it once again.
>

Just to add that the strategy above worked very well. :)