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 <rguent...@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 ----