[PATCH] Tree SRA and atomicity/volatility

[Eric Botcazou]

Hi,

The Ada language has the concept of atomicity, for example:

   type Word is
      record
         First, Second, Third, Fourth : Byte;
      end record;

   External : Word := (others => 0);
   pragma Atomic (External);

Quoting the Ada RM: "For an atomic object (including an atomic component) all 
reads and updates of the object as a whole are indivisible."

The GNAT compiler implements it by (among other things) declaring the object 
as being volatile.  While this works well in 3.x, this breaks in 4.x because 
the SRA may decompose accesses to volatile aggregates:

eric@linux:~/build/gcc/native> cat t.c
struct S
{
  char a;
  char b;
  char c;
  char d;
};

void foo(void)
{
  volatile struct S dev;
  struct S tmp = dev;
  tmp.a = 0;
  dev = tmp;
}

In .sra at -O:

;; Function foo (foo)

Cannot scalarize variable dev because it is declared volatile

[...]

foo ()
{
  char tmp$d;
  char tmp$c;
  char tmp$b;
  char tmp$a;
  struct S tmp;
  struct S dev;

<bb 2>:
  tmp$d_6 = dev.d;
  tmp$c_7 = dev.c;
  tmp$b_8 = dev.b;
  tmp$a_9 = dev.a;
  tmp$a_10 = 0;
  dev.d = tmp$d_6;
  dev.c = tmp$c_7;
  dev.b = tmp$b_8;
  dev.a = tmp$a_10;
  return;

}

but accesses to the volatile object are nevertheless decomposed.  On x86-64:

foo:
.LFB2:
        movzbl  -21(%rsp), %eax
        movzbl  -22(%rsp), %edx
        movzbl  -23(%rsp), %ecx
        movzbl  -24(%rsp), %esi
        movb    %al, -21(%rsp)
        movb    %dl, -22(%rsp)
        movb    %cl, -23(%rsp)
        movb    $0, -24(%rsp)
        ret


While this may or may not be a problem for the C language, I think that it's a 
real bug in the Tree SRA pass, which doesn't check whether the object to be 
decomposed has side-effects.  More specifically, this is the LDST case:

  /* Invoked when we have a copy between one scalarizable reference ELT
     and one non-scalarizable reference OTHER.  IS_OUTPUT is true if ELT
     is on the left-hand side.  */
  void (*ldst) (struct sra_elt *elt, tree other,
		block_stmt_iterator *bsi, bool is_output);

/* Scalarize a LDST.  To recap, this is an assignment between one scalarizable
   reference ELT and one non-scalarizable reference OTHER.  IS_OUTPUT is true
   if ELT is on the left-hand side.  */

static void
scalarize_ldst (struct sra_elt *elt, tree other,
		block_stmt_iterator *bsi, bool is_output)
{
  /* Shouldn't have gotten called for a scalar.  */
  gcc_assert (!elt->replacement);

  if (elt->use_block_copy)
    {
      /* Since ELT is not fully instantiated, we have to leave the
	 block copy in place.  Treat this as a USE.  */
      scalarize_use (elt, NULL, bsi, is_output, false);
    }
  else
    {
      /* The interesting case is when ELT is fully instantiated.  In this
	 case we can have each element stored/loaded directly to/from the
	 corresponding slot in OTHER.  This avoids a block copy.  */

The last comment is wrong if OTHER has side-effects.  Hence the attached 
patch, tested on x86_64-suse-linux, which yields on x86-64:

foo:
.LFB2:
        movl    -24(%rsp), %eax
        movb    $0, %al
        movl    %eax, -24(%rsp)
        ret


This is a regression from the 3.x series for the Ada compiler so I'm 
requesting approval for mainline/4.2/4.1 branches.


2007-01-06  Eric Botcazou  <ebotcazou@adacore.com>

	* tree-sra.c (sra_walk_fns) <ldst>: Document new restriction.
	(sra_walk_modify_expr) <rhs_elt>: Treat the reference as a use
	if the lhs has side-effects.
	<lhs_elt>: Treat the reference as a use if the rhs has side-effects.



:ADDPATCH Tree-SRA:

-- 
Eric Botcazou
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fc04-007_fsf.diff
Type: text/x-diff
Size: 1606 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20070106/55d132a7/attachment.bin>