latch mem to reg before multi-access in convert_move

latch mem to reg before multi-access in convert_move

[Olivier Hainque]

[-- Attachment #1: Type: text/plain, Size: 1422 bytes --]

Hello,

At some point in convert_move we have:

<<    /* No special multiword conversion insn; do it by hand.  */

      start_sequence ();

      /* Since we will turn this into a no conflict block, we must ensure
	 that the source does not overlap the target.  */

      if (reg_overlap_mentioned_p (to, from))
	from = force_reg (from_mode, from);
 
      ... emit a multi-insn sequence ...
>>

With a previous version of the compiler, we happened to get there
pretty easily with MEM_P(from) and the multi-insn sequence producing
multiple memory references to the source.

The original problem we had with this was the introduction of an
artificial race condition in addition to the potential performance
impact.

We had fixed this with the attached patch, which simply forces "from"
into a REG if it is a MEM, in addition to the current condition.
While our original testcases don't expose the problem with current
versions of the compiler, the issue appears to remain latent and the
change still looks sensible in any case.

We have been running with it for years across several versions of 
gcc on numerous platforms. I have just bootstrapped and regtested
with mainline on x86_64-linux.

OK to commit ?

Thanks in advance,

Olivier

2012-04056  Olivier Hainque  <hainque@adacore.com>

        * expr.c (convert_move): Latch mem integer inputs into a
        register before expanding a multi-instructions sequence.


[-- Attachment #2: memlatch.dif --]
[-- Type: video/x-dv, Size: 1240 bytes --]

[-- Attachment #3: Type: text/plain, Size: 2 bytes --]

ping: latch mem to reg before multi-access in convert_move

[Olivier Hainque]

Hello,

Ping for http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00298.html

Thanks in advance,

Olivier

On Apr 5, 2012, at 17:30 , Olivier Hainque wrote:
...
> The original problem we had with this was the introduction of an
> artificial race condition in addition to the potential performance
> impact.
> 
> We had fixed this with the attached patch, which simply forces "from"
> into a REG if it is a MEM, in addition to the current condition.
> While our original testcases don't expose the problem with current
> versions of the compiler, the issue appears to remain latent and the
> change still looks sensible in any case.
...
>        * expr.c (convert_move): Latch mem integer inputs into a
>        register before expanding a multi-instructions sequence.

ping x2 : latch mem to reg before multi-access in convert_move

[Olivier Hainque]

Hello,

Ping # 2 for http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00298.html

Thanks in advance,

Olivier

(sorry for a possible empty version of that message
sent by mistake a few minutes ago)

On Apr 5, 2012, at 17:30 , Olivier Hainque wrote:
...
> With a previous version of the compiler, we happened to get there
> pretty easily with MEM_P(from) and the multi-insn sequence producing
> multiple memory references to the source.
> 
> The original problem we had with this was the introduction of an
> artificial race condition in addition to the potential performance
> impact.

> While our original testcases don't expose the problem with current
> versions of the compiler, the issue appears to remain latent and the
> change still looks sensible in any case.
...
>        * expr.c (convert_move): Latch mem integer inputs into a
>        register before expanding a multi-instructions sequence.
> 
> <memlatch.dif>
> 

ping x3 : latch mem to reg before multi-access in convert_move

[Olivier Hainque]

Hello,

ping # 3 for http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00298.html

This is related to convert-move possibly emitting a sequence
with multiple accesses to one input, triggering multiple memory
accesses when that input happens to be a mem.

> The original problem we had with this was the introduction of an
> artificial race condition in addition to the potential performance
> impact.

> While our original testcases don't expose the problem with current
> versions of the compiler, the issue appears to remain latent and the
> change still looks sensible in any case.
...
>       * expr.c (convert_move): Latch mem integer inputs into a
>       register before expanding a multi-instructions sequence.

Thanks in advance,

Olivier

Re: ping x3 : latch mem to reg before multi-access in convert_move

[Richard Henderson]

On 2012-06-23 23:56, Olivier Hainque wrote:
> Hello,
> 
> ping # 3 for http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00298.html
> 
> This is related to convert-move possibly emitting a sequence
> with multiple accesses to one input, triggering multiple memory
> accesses when that input happens to be a mem.

Can you explain the sequence that results in multiple memory
references?  Because I can't see how it can...


r~

Re: ping x3 : latch mem to reg before multi-access in convert_move

[Olivier Hainque]

Hello Richard,

On Jun 25, 2012, at 20:36 , Richard Henderson wrote:

> Can you explain the sequence that results in multiple memory
> references?  Because I can't see how it can...

 Compared to the point at which we were observing the problem,
 the code has changed a bit in this area though not so much.

 In the current mainline, this would be from the two uses of
 "lowfrom" in:

      /* Get a copy of FROM widened to a word, if necessary.  */
      ...
      lowfrom = convert_to_mode (lowpart_mode, from, unsignedp);

      lowpart = gen_lowpart (lowpart_mode, to);
 ==>  emit_move_insn (lowpart, lowfrom);

      /* Compute the value to put in each remaining word.  */
      if (unsignedp)
        fill_value = const0_rtx;
      else
        fill_value = emit_store_flag (gen_reg_rtx (word_mode),
 ==>                                  LT, lowfrom, const0_rtx,
                                      VOIDmode, 0, -1);

Re: ping x3 : latch mem to reg before multi-access in convert_move

[Richard Henderson]

On 06/26/2012 03:40 AM, Olivier Hainque wrote:
> Hello Richard,
> 
> On Jun 25, 2012, at 20:36 , Richard Henderson wrote:
> 
>> Can you explain the sequence that results in multiple memory
>> references?  Because I can't see how it can...
> 
>  Compared to the point at which we were observing the problem,
>  the code has changed a bit in this area though not so much.
> 
>  In the current mainline, this would be from the two uses of
>  "lowfrom" in:
> 
>       /* Get a copy of FROM widened to a word, if necessary.  */
>       ...
>       lowfrom = convert_to_mode (lowpart_mode, from, unsignedp);
> 
>       lowpart = gen_lowpart (lowpart_mode, to);
>  ==>  emit_move_insn (lowpart, lowfrom);
> 
>       /* Compute the value to put in each remaining word.  */
>       if (unsignedp)
>         fill_value = const0_rtx;
>       else
>         fill_value = emit_store_flag (gen_reg_rtx (word_mode),
>  ==>                                  LT, lowfrom, const0_rtx,
>                                       VOIDmode, 0, -1);

Ah, right, since convert_to_mode can just reshuffle the MEM.

The patch is ok.


r~

Re: ping x3 : latch mem to reg before multi-access in convert_move

[Olivier Hainque]

On Jun 26, 2012, at 17:28 , Richard Henderson wrote:

> Ah, right, since convert_to_mode can just reshuffle the MEM.
> 
> The patch is ok.

 Great, will commit shortly.

 Thanks a lot for your prompt feedback,

 With Kind Regards,

 Olivier