[PATCH] Fix PR middle-end/26306

[Eric Botcazou <ebotcazou at adacore dot com>]

Hi,

This is a regression present in every compiler of the 4.x series.  The 
compiler aborts on the attached Ada testcase because it is trying to
force a load of a volatile aggregate object with variable size.

The problematic bits are in the gimplifier:

      else if (COMPLETE_TYPE_P (TREE_TYPE (*expr_p)))
	{
	  /* Historically, the compiler has treated a bare
	     reference to a volatile lvalue as forcing a load.  */

That's partially wrong.  On the following C testcase:

struct S { double d; int i; };

int count;

void tick (void)
{
  count++;
}

volatile int a;

void foo(void)
{
  volatile struct S arr[4];

  (void) arr[tick (), a];
}

the 3.4.6 compiler generates at -O0 on x86:

foo:
	pushl	%ebp
	movl	%esp, %ebp
	subl	$56, %esp
	call	tick
	movl	a, %eax
	leave
	ret

The relevant bits in the RTL expander are:

      /* Ensure we reference a volatile object even if value is ignored, but
	 don't do this if all we are doing is taking its address.  */
      if (TREE_THIS_VOLATILE (exp)
	  && TREE_CODE (exp) != FUNCTION_DECL
	  && mode != VOIDmode && mode != BLKmode
	  && modifier != EXPAND_CONST_ADDRESS)
	{
	  temp = expand_expr (exp, NULL_RTX, VOIDmode, modifier);
	  if (MEM_P (temp))
	    temp = copy_to_reg (temp);
	  return const0_rtx;
	}

Of course the crucial difference is the missing equivalent of

  mode != BLKmode

in the gimplifier.  Hence the proposed fix.

Tested on x86_64-suse-linux.  OK for mainline/4.2/4.1 branches?


2006-11-05  Eric Botcazou  <ebotcazou@adacore.com>

	PR middle-end/26306
	* gimplify.c (gimplify_expr): Only force a load for references to
	volatile scalar values.


2006-11-05  Eric Botcazou  <ebotcazou@adacore.com>

	* gnat.dg/volatile_aggregate.adb: New test.


:ADDPATCH middle-end:


-- 
Eric Botcazou

Index: gimplify.c
===================================================================
--- gimplify.c	(revision 118450)
+++ gimplify.c	(working copy)
@@ -5852,7 +5852,8 @@ gimplify_expr (tree *expr_p, tree *pre_p
 			     gimple_test_f, fallback);
 	      break;
 
-	    case ARRAY_REF: case ARRAY_RANGE_REF:
+	    case ARRAY_REF:
+	    case ARRAY_RANGE_REF:
 	      gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p,
 			     gimple_test_f, fallback);
 	      gimplify_expr (&TREE_OPERAND (*expr_p, 1), pre_p, post_p,
@@ -5861,16 +5862,17 @@ gimplify_expr (tree *expr_p, tree *pre_p
 
 	    default:
 	       /* Anything else with side-effects must be converted to
-	       	  a valid statement before we get here.  */
+		  a valid statement before we get here.  */
 	      gcc_unreachable ();
 	    }
 
 	  *expr_p = NULL;
 	}
-      else if (COMPLETE_TYPE_P (TREE_TYPE (*expr_p)))
+      else if (COMPLETE_TYPE_P (TREE_TYPE (*expr_p))
+	       && !AGGREGATE_TYPE_P (TREE_TYPE (*expr_p)))
 	{
-	  /* Historically, the compiler has treated a bare
-	     reference to a volatile lvalue as forcing a load.  */
+	  /* Historically, the compiler has treated a bare reference
+	     to a volatile scalar lvalue as forcing a load.  */
 	  tree type = TYPE_MAIN_VARIANT (TREE_TYPE (*expr_p));
 	  /* Normally, we do not want to create a temporary for a
 	     TREE_ADDRESSABLE type because such a type should not be
@@ -5885,7 +5887,10 @@ gimplify_expr (tree *expr_p, tree *pre_p
 	}
       else
 	/* We can't do anything useful with a volatile reference to
-	   incomplete type, so just throw it away.  */
+	   an incomplete type, so just throw it away.  Likewise for
+	   an aggregate type, since any implicit inner load should
+	   already have been turned into an explicit one by the
+	   gimplification process.  */
 	*expr_p = NULL;
     }
 

-- { dg-do compile }

with System;

procedure Volatile_Aggregate is 

  function GetArrayUpperBound return Integer is 
  begin
    return 2;
  end GetArrayUpperBound; 

  some_value : Integer := GetArrayUpperBound;

  type Gp_Element_Type is record
    Element : Integer;
  end record;

  type some_type is array (1 .. some_value) of Gp_Element_Type;

  type Aligned_Some_Type is record
    Value : aliased some_type;
  end record;          

  for Aligned_Some_Type'Alignment use 8;          

  an_aligned_type : aligned_Some_Type;   
  my_address : system.address; 

  pragma Volatile (an_aligned_type);

begin
  my_address := an_aligned_type.value(1)'address; 
end;