[PATCH] Re-work get_object_alignment (again)

[PATCH] Re-work get_object_alignment (again)

[Richard Guenther]

I've arrived at get_object_{or_type,}alignment again looking at
PR53970.  And I finally concluded we should unconditionally relying
on type-alignment on INDIRECT/MEM/TARGET_MEM_REF when we ask for
the alignment of an access (as opposed to when asking for the
alignment of an address).  So the following patch removes
get_object_or_type_alignment again and folds its doings into
get_object_alignment_2 which now takes an argument to indicate
whether we compute address or object alignment.

I changed the meaning of the return value of the functions to
indicate whether the information returned is "correct".  That's
used to disable the use of type-alignment knowledge when we
know for example the underlying decl and see it is not appropriately
aligned.  This should mask some of the bugs in user code and
in the C frontend (at least) with regarding to packed structs.

I have bootstrapped and tested this patch on x86_64-unknown-linux-gnu
and also a variant that asserts that the alignment returned by
get_object_alignment is at least as large as that computed by
get_object_or_type_alignment (so we should not regress).

Now, back to PR53970, where #pragma pack() is used to pack a
struct.  With #pragma pack() no part of the type or field-decls
have a hint that packing took place (well, their align information
tell you), which means the vectorizers use of contains_packed_reference
is not conservative enough, nor is expand_exprs use:

    case BIT_FIELD_REF:
    case ARRAY_RANGE_REF:
    normal_inner_ref:
      {
...
        if (TYPE_PACKED (TREE_TYPE (TREE_OPERAND (exp, 0)))
            || (TREE_CODE (TREE_OPERAND (exp, 1)) == FIELD_DECL
                && DECL_PACKED (TREE_OPERAND (exp, 1))))
          packedp = true;

I'm not sure if this flag is required for correctness - it's only
passed to extract_bit_field - but if it is the above code does
not work for #pragma packed structs.  I suppose what should be
checked is (a few lines below the above test, after we expanded tem)

        if (MEM_P (op0)
            && GET_MODE (op0) != BLKmode
	    && MEM_ALIGN (op0) < GET_MODE_ALIGNMENT (GET_MODE (op0)))
          packedp = true;

?  I suppose packedp was computed for STRICT_ALIGNMENT targets only.
I'm not changing the above, but Eric, you should be able to produce a
#pragma packed testcase that fails on a STRICT_ALIGNMENT target?

Comments welcome, of course.

Oh, and this does not yet fix PR53970 - but I hope that I can
remove contains_packed_reference ;)

Thanks,
Richard.

2012-07-17  Richard Guenther  <rguenther@suse.de>

	* tree.h (get_object_or_type_alignment): Remove.
	* builtins.c (get_object_alignment_2): New function copied from
	get_object_alignment_1.  Take extra argument to indicate whether
	we take the address of EXP.  Rework to use type alignment information
	if not, and return whether the result is an approximation or not.
	(get_object_alignment_1): Wrap around get_object_alignment_2.
	(get_pointer_alignment_1): Call get_object_alignment_2 indicating
	we take the address.
	(get_object_or_type_alignment): Remove.
	* expr.c (expand_assignment): Call get_object_alignment.
	(expand_expr_real_1): Likewise.

Index: gcc/builtins.c
===================================================================
*** gcc/builtins.c.orig	2012-07-17 12:26:37.000000000 +0200
--- gcc/builtins.c	2012-07-17 13:15:19.018757760 +0200
*************** called_as_built_in (tree node)
*** 273,283 ****
     on the address at which an object is actually located.  These two
     addresses are not always the same.  For example, on ARM targets,
     the address &foo of a Thumb function foo() has the lowest bit set,
!    whereas foo() itself starts on an even address.  */
  
! bool
! get_object_alignment_1 (tree exp, unsigned int *alignp,
! 			unsigned HOST_WIDE_INT *bitposp)
  {
    HOST_WIDE_INT bitsize, bitpos;
    tree offset;
--- 273,286 ----
     on the address at which an object is actually located.  These two
     addresses are not always the same.  For example, on ARM targets,
     the address &foo of a Thumb function foo() has the lowest bit set,
!    whereas foo() itself starts on an even address.
  
!    If ADDR_P is true we are taking the address of the memory reference EXP
!    and thus cannot rely on the access taking place.  */
! 
! static bool
! get_object_alignment_2 (tree exp, unsigned int *alignp,
! 			unsigned HOST_WIDE_INT *bitposp, bool addr_p)
  {
    HOST_WIDE_INT bitsize, bitpos;
    tree offset;
*************** get_object_alignment_1 (tree exp, unsign
*** 293,340 ****
  
    /* Extract alignment information from the innermost object and
       possibly adjust bitpos and offset.  */
!   if (TREE_CODE (exp) == CONST_DECL)
!     exp = DECL_INITIAL (exp);
!   if (DECL_P (exp)
!       && TREE_CODE (exp) != LABEL_DECL)
      {
!       if (TREE_CODE (exp) == FUNCTION_DECL)
! 	{
! 	  /* Function addresses can encode extra information besides their
! 	     alignment.  However, if TARGET_PTRMEMFUNC_VBIT_LOCATION
! 	     allows the low bit to be used as a virtual bit, we know
! 	     that the address itself must be 2-byte aligned.  */
! 	  if (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn)
! 	    {
! 	      known_alignment = true;
! 	      align = 2 * BITS_PER_UNIT;
! 	    }
! 	}
!       else
! 	{
! 	  known_alignment = true;
! 	  align = DECL_ALIGN (exp);
! 	}
!     }
!   else if (CONSTANT_CLASS_P (exp))
      {
!       known_alignment = true;
        align = TYPE_ALIGN (TREE_TYPE (exp));
  #ifdef CONSTANT_ALIGNMENT
!       align = (unsigned)CONSTANT_ALIGNMENT (exp, align);
  #endif
      }
!   else if (TREE_CODE (exp) == VIEW_CONVERT_EXPR)
      {
        known_alignment = true;
-       align = TYPE_ALIGN (TREE_TYPE (exp));
      }
!   else if (TREE_CODE (exp) == INDIRECT_REF)
      {
-       known_alignment = true;
        align = TYPE_ALIGN (TREE_TYPE (exp));
      }
!   else if (TREE_CODE (exp) == MEM_REF)
      {
        tree addr = TREE_OPERAND (exp, 0);
        unsigned ptr_align;
--- 296,335 ----
  
    /* Extract alignment information from the innermost object and
       possibly adjust bitpos and offset.  */
!   if (TREE_CODE (exp) == FUNCTION_DECL)
      {
!       /* Function addresses can encode extra information besides their
! 	 alignment.  However, if TARGET_PTRMEMFUNC_VBIT_LOCATION
! 	 allows the low bit to be used as a virtual bit, we know
! 	 that the address itself must be at least 2-byte aligned.  */
!       if (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn)
! 	align = 2 * BITS_PER_UNIT;
!     }
!   else if (TREE_CODE (exp) == LABEL_DECL)
!     ;
!   else if (TREE_CODE (exp) == CONST_DECL)
      {
!       /* The alignment of a CONST_DECL is determined by its initializer.  */
!       exp = DECL_INITIAL (exp);
        align = TYPE_ALIGN (TREE_TYPE (exp));
  #ifdef CONSTANT_ALIGNMENT
!       if (CONSTANT_CLASS_P (exp))
! 	align = (unsigned) CONSTANT_ALIGNMENT (exp, align);
  #endif
+       known_alignment = true;
      }
!   else if (DECL_P (exp))
      {
+       align = DECL_ALIGN (exp);
        known_alignment = true;
      }
!   else if (TREE_CODE (exp) == VIEW_CONVERT_EXPR)
      {
        align = TYPE_ALIGN (TREE_TYPE (exp));
      }
!   else if (TREE_CODE (exp) == INDIRECT_REF
! 	   || TREE_CODE (exp) == MEM_REF
! 	   || TREE_CODE (exp) == TARGET_MEM_REF)
      {
        tree addr = TREE_OPERAND (exp, 0);
        unsigned ptr_align;
*************** get_object_alignment_1 (tree exp, unsign
*** 343,400 ****
        if (TREE_CODE (addr) == BIT_AND_EXPR
  	  && TREE_CODE (TREE_OPERAND (addr, 1)) == INTEGER_CST)
  	{
- 	  known_alignment = true;
  	  align = (TREE_INT_CST_LOW (TREE_OPERAND (addr, 1))
  		    & -TREE_INT_CST_LOW (TREE_OPERAND (addr, 1)));
  	  align *= BITS_PER_UNIT;
  	  addr = TREE_OPERAND (addr, 0);
  	}
  
!       if (get_pointer_alignment_1 (addr, &ptr_align, &ptr_bitpos))
  	{
! 	  known_alignment = true;
! 	  bitpos += ptr_bitpos & ~(align - 1);
! 	  align = MAX (ptr_align, align);
  	}
  
!       bitpos += mem_ref_offset (exp).low * BITS_PER_UNIT;
      }
!   else if (TREE_CODE (exp) == TARGET_MEM_REF)
      {
!       unsigned ptr_align;
!       unsigned HOST_WIDE_INT ptr_bitpos;
!       tree addr = TMR_BASE (exp);
! 
!       if (TREE_CODE (addr) == BIT_AND_EXPR
! 	  && TREE_CODE (TREE_OPERAND (addr, 1)) == INTEGER_CST)
! 	{
! 	  known_alignment = true;
! 	  align = (TREE_INT_CST_LOW (TREE_OPERAND (addr, 1))
! 		   & -TREE_INT_CST_LOW (TREE_OPERAND (addr, 1)));
! 	  align *= BITS_PER_UNIT;
! 	  addr = TREE_OPERAND (addr, 0);
! 	}
! 
!       if (get_pointer_alignment_1 (addr, &ptr_align, &ptr_bitpos))
! 	{
! 	  known_alignment = true;
! 	  bitpos += ptr_bitpos & ~(align - 1);
! 	  align = MAX (ptr_align, align);
! 	}
! 
!       if (TMR_OFFSET (exp))
! 	bitpos += TREE_INT_CST_LOW (TMR_OFFSET (exp)) * BITS_PER_UNIT;
!       if (TMR_INDEX (exp) && TMR_STEP (exp))
! 	{
! 	  unsigned HOST_WIDE_INT step = TREE_INT_CST_LOW (TMR_STEP (exp));
! 	  align = MIN (align, (step & -step) * BITS_PER_UNIT);
! 	  known_alignment = true;
! 	}
!       else if (TMR_INDEX (exp))
! 	known_alignment = false;
! 
!       if (TMR_INDEX2 (exp))
! 	known_alignment = false;
      }
  
    /* If there is a non-constant offset part extract the maximum
--- 338,388 ----
        if (TREE_CODE (addr) == BIT_AND_EXPR
  	  && TREE_CODE (TREE_OPERAND (addr, 1)) == INTEGER_CST)
  	{
  	  align = (TREE_INT_CST_LOW (TREE_OPERAND (addr, 1))
  		    & -TREE_INT_CST_LOW (TREE_OPERAND (addr, 1)));
  	  align *= BITS_PER_UNIT;
  	  addr = TREE_OPERAND (addr, 0);
  	}
  
!       known_alignment
! 	= get_pointer_alignment_1 (addr, &ptr_align, &ptr_bitpos);
!       bitpos += ptr_bitpos;
!       align = MAX (ptr_align, align);
! 
!       if (TREE_CODE (exp) == MEM_REF
! 	  || TREE_CODE (exp) == TARGET_MEM_REF)
! 	bitpos += mem_ref_offset (exp).low * BITS_PER_UNIT;
!       if (TREE_CODE (exp) == TARGET_MEM_REF)
  	{
! 	  if (TMR_INDEX (exp))
! 	    {
! 	      unsigned HOST_WIDE_INT step = 1;
! 	      if (TMR_STEP (exp))
! 		step = TREE_INT_CST_LOW (TMR_STEP (exp));
! 	      align = MIN (align, (step & -step) * BITS_PER_UNIT);
! 	    }
! 	  if (TMR_INDEX2 (exp))
! 	    align = BITS_PER_UNIT;
! 	  known_alignment = false;
  	}
  
!       /* When EXP is an actual memory reference then we can use
! 	 TYPE_ALIGN of a pointer indirection to derive alignment.
! 	 Do so only if get_pointer_alignment_1 did not reveal absolute
! 	 alignment knowledge.  */
!       if (!addr_p && !known_alignment)
! 	align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align);
      }
!   else if (TREE_CODE (exp) == STRING_CST)
      {
!       /* STRING_CST are the only constant objects we allow to be not
!          wrapped inside a CONST_DECL.  */
!       align = TYPE_ALIGN (TREE_TYPE (exp));
! #ifdef CONSTANT_ALIGNMENT
!       if (CONSTANT_CLASS_P (exp))
! 	align = (unsigned) CONSTANT_ALIGNMENT (exp, align);
! #endif
!       known_alignment = true;
      }
  
    /* If there is a non-constant offset part extract the maximum
*************** get_object_alignment_1 (tree exp, unsign
*** 435,463 ****
  	}
        else
  	{
! 	  known_alignment = false;
  	  break;
  	}
        offset = next_offset;
      }
  
!   if (known_alignment)
!     {
!       /* Alignment is innermost object alignment adjusted by the constant
! 	 and non-constant offset parts.  */
!       align = MIN (align, inner);
!       bitpos = bitpos & (align - 1);
!       *alignp = align;
!     }
!   else
!     {
!       bitpos = bitpos & (BITS_PER_UNIT - 1);
!       *alignp = BITS_PER_UNIT;
!     }
!   *bitposp = bitpos;
    return known_alignment;
  }
  
  /* Return the alignment in bits of EXP, an object.  */
  
  unsigned int
--- 423,454 ----
  	}
        else
  	{
! 	  inner = MIN (inner, BITS_PER_UNIT);
  	  break;
  	}
        offset = next_offset;
      }
+   /* Alignment is innermost object alignment adjusted by the constant
+      and non-constant offset parts.  */
+   align = MIN (align, inner);
  
!   *alignp = align;
!   *bitposp = bitpos & (*alignp - 1);
    return known_alignment;
  }
  
+ /* For a memory reference expression EXP compute values M and N such that M
+    divides (&EXP - N) and such that N < M.  If these numbers can be determined,
+    store M in alignp and N in *BITPOSP and return true.  Otherwise return false
+    and store BITS_PER_UNIT to *alignp and any bit-offset to *bitposp.  */
+ 
+ bool
+ get_object_alignment_1 (tree exp, unsigned int *alignp,
+ 			unsigned HOST_WIDE_INT *bitposp)
+ {
+   return get_object_alignment_2 (exp, alignp, bitposp, false);
+ }
+ 
  /* Return the alignment in bits of EXP, an object.  */
  
  unsigned int
*************** get_object_alignment (tree exp)
*** 476,511 ****
    return align;
  }
  
- /* Return the alignment of object EXP, also considering its type when we do
-    not know of explicit misalignment.  Only handle MEM_REF and TARGET_MEM_REF.
- 
-    ??? Note that, in the general case, the type of an expression is not kept
-    consistent with misalignment information by the front-end, for example when
-    taking the address of a member of a packed structure.  However, in most of
-    the cases, expressions have the alignment of their type so we optimistically
-    fall back to this alignment when we cannot compute a misalignment.  */
- 
- unsigned int
- get_object_or_type_alignment (tree exp)
- {
-   unsigned HOST_WIDE_INT misalign;
-   unsigned int align;
-   bool known_alignment;
- 
-   gcc_assert (TREE_CODE (exp) == MEM_REF || TREE_CODE (exp) == TARGET_MEM_REF);
-   known_alignment = get_object_alignment_1 (exp, &align, &misalign);
-   if (misalign != 0)
-     align = (misalign & -misalign);
-   else if (!known_alignment)
-     align = TYPE_ALIGN (TREE_TYPE (exp));
- 
-   return align;
- }
- 
  /* For a pointer valued expression EXP compute values M and N such that M
     divides (EXP - N) and such that N < M.  If these numbers can be determined,
!    store M in alignp and N in *BITPOSP and return true.  Otherwise return false
!    and store BITS_PER_UNIT to *alignp and any bit-offset to *bitposp.
  
     If EXP is not a pointer, false is returned too.  */
  
--- 467,476 ----
    return align;
  }
  
  /* For a pointer valued expression EXP compute values M and N such that M
     divides (EXP - N) and such that N < M.  If these numbers can be determined,
!    store M in alignp and N in *BITPOSP and return true.  Return false if
!    the results are just a conservative approximation.
  
     If EXP is not a pointer, false is returned too.  */
  
*************** get_pointer_alignment_1 (tree exp, unsig
*** 516,522 ****
    STRIP_NOPS (exp);
  
    if (TREE_CODE (exp) == ADDR_EXPR)
!     return get_object_alignment_1 (TREE_OPERAND (exp, 0), alignp, bitposp);
    else if (TREE_CODE (exp) == SSA_NAME
  	   && POINTER_TYPE_P (TREE_TYPE (exp)))
      {
--- 481,488 ----
    STRIP_NOPS (exp);
  
    if (TREE_CODE (exp) == ADDR_EXPR)
!     return get_object_alignment_2 (TREE_OPERAND (exp, 0),
! 				   alignp, bitposp, true);
    else if (TREE_CODE (exp) == SSA_NAME
  	   && POINTER_TYPE_P (TREE_TYPE (exp)))
      {
*************** get_pointer_alignment_1 (tree exp, unsig
*** 527,532 ****
--- 493,499 ----
  	{
  	  *bitposp = ptr_misalign * BITS_PER_UNIT;
  	  *alignp = ptr_align * BITS_PER_UNIT;
+ 	  /* We cannot really tell whether this result is an approximation.  */
  	  return true;
  	}
        else
Index: gcc/expr.c
===================================================================
*** gcc/expr.c.orig	2012-07-16 16:22:49.000000000 +0200
--- gcc/expr.c	2012-07-17 13:16:05.641756144 +0200
*************** expand_assignment (tree to, tree from, b
*** 4590,4596 ****
         || TREE_CODE (to) == TARGET_MEM_REF)
        && mode != BLKmode
        && !mem_ref_refers_to_non_mem_p (to)
!       && ((align = get_object_or_type_alignment (to))
  	  < GET_MODE_ALIGNMENT (mode))
        && (((icode = optab_handler (movmisalign_optab, mode))
  	   != CODE_FOR_nothing)
--- 4590,4596 ----
         || TREE_CODE (to) == TARGET_MEM_REF)
        && mode != BLKmode
        && !mem_ref_refers_to_non_mem_p (to)
!       && ((align = get_object_alignment (to))
  	  < GET_MODE_ALIGNMENT (mode))
        && (((icode = optab_handler (movmisalign_optab, mode))
  	   != CODE_FOR_nothing)
*************** expand_assignment (tree to, tree from, b
*** 4652,4658 ****
        mode = TYPE_MODE (TREE_TYPE (tem));
        if (TREE_CODE (tem) == MEM_REF
  	  && mode != BLKmode
! 	  && ((align = get_object_or_type_alignment (tem))
  	      < GET_MODE_ALIGNMENT (mode))
  	  && ((icode = optab_handler (movmisalign_optab, mode))
  	      != CODE_FOR_nothing))
--- 4652,4658 ----
        mode = TYPE_MODE (TREE_TYPE (tem));
        if (TREE_CODE (tem) == MEM_REF
  	  && mode != BLKmode
! 	  && ((align = get_object_alignment (tem))
  	      < GET_MODE_ALIGNMENT (mode))
  	  && ((icode = optab_handler (movmisalign_optab, mode))
  	      != CODE_FOR_nothing))
*************** expand_expr_real_1 (tree exp, rtx target
*** 9496,9502 ****
  	temp = gen_rtx_MEM (mode, op0);
  	set_mem_attributes (temp, exp, 0);
  	set_mem_addr_space (temp, as);
! 	align = get_object_or_type_alignment (exp);
  	if (modifier != EXPAND_WRITE
  	    && mode != BLKmode
  	    && align < GET_MODE_ALIGNMENT (mode)
--- 9496,9502 ----
  	temp = gen_rtx_MEM (mode, op0);
  	set_mem_attributes (temp, exp, 0);
  	set_mem_addr_space (temp, as);
! 	align = get_object_alignment (exp);
  	if (modifier != EXPAND_WRITE
  	    && mode != BLKmode
  	    && align < GET_MODE_ALIGNMENT (mode)
*************** expand_expr_real_1 (tree exp, rtx target
*** 9570,9576 ****
  			   gimple_assign_rhs1 (def_stmt), mask);
  	    TREE_OPERAND (exp, 0) = base;
  	  }
! 	align = get_object_or_type_alignment (exp);
  	op0 = expand_expr (base, NULL_RTX, VOIDmode, EXPAND_SUM);
  	op0 = memory_address_addr_space (address_mode, op0, as);
  	if (!integer_zerop (TREE_OPERAND (exp, 1)))
--- 9570,9576 ----
  			   gimple_assign_rhs1 (def_stmt), mask);
  	    TREE_OPERAND (exp, 0) = base;
  	  }
! 	align = get_object_alignment (exp);
  	op0 = expand_expr (base, NULL_RTX, VOIDmode, EXPAND_SUM);
  	op0 = memory_address_addr_space (address_mode, op0, as);
  	if (!integer_zerop (TREE_OPERAND (exp, 1)))
Index: gcc/tree.h
===================================================================
*** gcc/tree.h.orig	2012-07-17 13:15:41.000000000 +0200
--- gcc/tree.h	2012-07-17 13:15:50.070756591 +0200
*************** extern bool is_builtin_fn (tree);
*** 5485,5491 ****
  extern bool get_object_alignment_1 (tree, unsigned int *,
  				    unsigned HOST_WIDE_INT *);
  extern unsigned int get_object_alignment (tree);
- extern unsigned int get_object_or_type_alignment (tree);
  extern bool get_pointer_alignment_1 (tree, unsigned int *,
  				     unsigned HOST_WIDE_INT *);
  extern unsigned int get_pointer_alignment (tree);
--- 5485,5490 ----

Re: [PATCH] Re-work get_object_alignment (again)

[Eric Botcazou]

> Now, back to PR53970, where #pragma pack() is used to pack a
> struct.  With #pragma pack() no part of the type or field-decls
> have a hint that packing took place (well, their align information
> tell you), which means the vectorizers use of contains_packed_reference
> is not conservative enough, nor is expand_exprs use:
> 
>     case BIT_FIELD_REF:
>     case ARRAY_RANGE_REF:
>     normal_inner_ref:
>       {
> ...
>         if (TYPE_PACKED (TREE_TYPE (TREE_OPERAND (exp, 0)))
> 
>             || (TREE_CODE (TREE_OPERAND (exp, 1)) == FIELD_DECL
> 
>                 && DECL_PACKED (TREE_OPERAND (exp, 1))))
>           packedp = true;
> 
> I'm not sure if this flag is required for correctness - it's only
> passed to extract_bit_field - but if it is the above code does
> not work for #pragma packed structs.  I suppose what should be
> checked is (a few lines below the above test, after we expanded tem)
> 
>         if (MEM_P (op0)
>             && GET_MODE (op0) != BLKmode
> 	    && MEM_ALIGN (op0) < GET_MODE_ALIGNMENT (GET_MODE (op0)))
>           packedp = true;
> 
> ?  I suppose packedp was computed for STRICT_ALIGNMENT targets only.
> I'm not changing the above, but Eric, you should be able to produce a
> #pragma packed testcase that fails on a STRICT_ALIGNMENT target?

This is the -fstrict-volatile-bitfields business though, and its documentation 
explicitly refers to the packed attribute:

     If the target requires strict alignment, and honoring the field
     type would require violating this alignment, a warning is issued.
     If the field has `packed' attribute, the access is done without
     honoring the field type.  If the field doesn't have `packed'
     attribute, the access is done honoring the field type.  In both
     cases, GCC assumes that the user knows something about the target
     hardware that it is unaware of.

so I'm a little reluctant to touch that.  But, yes, generally speaking, testing 
TYPE_PACKED or DECL_PACKED to drive code generation is wrong.

> Oh, and this does not yet fix PR53970 - but I hope that I can
> remove contains_packed_reference ;)

Right, it should definitely go away.

-- 
Eric Botcazou

Re: [PATCH] Re-work get_object_alignment (again)

[Markus Trippelsdorf]

On 2012.07.17 at 15:10 +0200, Richard Guenther wrote:
> Comments welcome, of course.

This patch apparently miscompiles the Linux kernel, which just
hangs during early boot:

...
SLUB: Genslabs=15, HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
Hierarchical RCU implementation.
NR_IRQS:4352 nr_irqs:712 16
Extended CMOS year: 2000
Console: colour VGA+ 80x25
console [tty0] enabled
(hang)

See: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54031

-- 
Markus

Re: [PATCH] Re-work get_object_alignment (again)

[Richard Guenther]

On Thu, 19 Jul 2012, Markus Trippelsdorf wrote:

> On 2012.07.17 at 15:10 +0200, Richard Guenther wrote:
> > Comments welcome, of course.
> 
> This patch apparently miscompiles the Linux kernel, which just
> hangs during early boot:
> 
> ...
> SLUB: Genslabs=15, HWalign=64, Order=0-3, MinObjects=0, CPUs=4, Nodes=1
> Hierarchical RCU implementation.
> NR_IRQS:4352 nr_irqs:712 16
> Extended CMOS year: 2000
> Console: colour VGA+ 80x25
> console [tty0] enabled
> (hang)
> 
> See: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54031

The following fixes one issue (but reportedly does not fix the above
issue).

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Richard.

2012-07-20  Richard Guenther  <rguenther@suse.de>

	* builtins.c (get_object_alignment_2): Correct offset handling
	when using type alignment of a MEM_REF kind base.

Index: gcc/builtins.c
===================================================================
--- gcc/builtins.c	(revision 189656)
+++ gcc/builtins.c	(working copy)
@@ -346,12 +346,10 @@ get_object_alignment_2 (tree exp, unsign
 
       known_alignment
 	= get_pointer_alignment_1 (addr, &ptr_align, &ptr_bitpos);
-      bitpos += ptr_bitpos;
       align = MAX (ptr_align, align);
 
-      if (TREE_CODE (exp) == MEM_REF
-	  || TREE_CODE (exp) == TARGET_MEM_REF)
-	bitpos += mem_ref_offset (exp).low * BITS_PER_UNIT;
+      /* The alignment of the pointer operand in a TARGET_MEM_REF
+	 has to take the variable offset parts into account.  */
       if (TREE_CODE (exp) == TARGET_MEM_REF)
 	{
 	  if (TMR_INDEX (exp))
@@ -369,9 +367,19 @@ get_object_alignment_2 (tree exp, unsign
       /* When EXP is an actual memory reference then we can use
 	 TYPE_ALIGN of a pointer indirection to derive alignment.
 	 Do so only if get_pointer_alignment_1 did not reveal absolute
-	 alignment knowledge.  */
-      if (!addr_p && !known_alignment)
-	align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align);
+	 alignment knowledge and if using that alignment would
+	 improve the situation.  */
+      if (!addr_p && !known_alignment
+	  && TYPE_ALIGN (TREE_TYPE (exp)) > align)
+	align = TYPE_ALIGN (TREE_TYPE (exp));
+      else
+	{
+	  /* Else adjust bitpos accordingly.  */
+	  bitpos += ptr_bitpos;
+	  if (TREE_CODE (exp) == MEM_REF
+	      || TREE_CODE (exp) == TARGET_MEM_REF)
+	    bitpos += mem_ref_offset (exp).low * BITS_PER_UNIT;
+	}
     }
   else if (TREE_CODE (exp) == STRING_CST)
     {