On Mon, 17 Oct 2011, Eric Botcazou wrote:
> > 2011-10-17 Richard Guenther <rguent...@suse.de>
> >
> > PR middle-end/50716
> > * expr.c (get_object_or_type_alignment): New function.
> > (expand_assignment): Use it.
> > (expand_expr_real_1): Likewise.
>
> Maybe move it to builtins.c alongside the other get_*_alignment functions.
I explicitly didn't do that because the function shouldn't be used
without knowing exactly what you do ;)
> > Index: gcc/expr.c
> > ===================================================================
> > *** gcc/expr.c (revision 180077)
> > --- gcc/expr.c (working copy)
> > *************** get_bit_range (unsigned HOST_WIDE_INT *b
> > *** 4544,4549 ****
> > --- 4544,4568 ----
> > }
> > }
> >
> > + /* Return the alignment of the object EXP, also considering its type
> > + when we do not know of explicit misalignment.
> > + ??? Note that generally the type of an expression is not kept
> > + consistent with misalignment information by the frontend, for
> > + example when taking the address of a member of a packed structure.
> > + So taking into account type information for alignment is generally
> > + wrong, but is done here as a compromise. */
>
> This sounds backwards, as taking into account type information is generally
> correct, i.e. the packed case is the exception. Maybe use "in the general
> case" instead:
>
> ??? Note that, in the general case, the type of an expression is not kept
> consistent with the misalignment 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 fall back to
> the alignment of the type when we cannot compute a misalignment.
Ok, I'll adjust it a bit, adding "so we fall back optimistically to
the alignment...", because that's what is true - for the packed case
we simply compute wrong alignment this way (but for the expand case
this has been the case since forever, noticed only on strict-align
targets).
Note that "cannot compute a misalignment" applies also to the
case where we for some reason explicitly know the alignment
is just BITS_PER_UNIT (you cannot 'misalign' that). So there are
still a lot of cases where known (in some sense of known) alignment
is overridden by maybe bogus type information. It's just less
cases now, but I think we may be able to improve the situation
by maybe returning an extra argument from get_object_alignment_1
that says whether the alignment might be bigger or whether the
alignment is definitively known. But that's for a followup I
presume.
>
> + static unsigned int
> + get_object_or_type_alignment (tree exp)
> + {
> + unsigned HOST_WIDE_INT misalign;
> + unsigned int align = get_object_alignment_1 (exp, &misalign);
> + align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align);
> + if (misalign != 0)
> + align = (misalign & -misalign);
>
> You don't need to go through the MAX if misalign is non-zero.
Ok.
Committed as follows.
Richard.
2011-10-18 Richard Guenther <rguent...@suse.de>
PR middle-end/50716
* expr.c (get_object_or_type_alignment): New function.
(expand_assignment): Use it.
(expand_expr_real_1): Likewise.
Index: gcc/expr.c
===================================================================
*** gcc/expr.c (revision 180077)
--- gcc/expr.c (working copy)
*************** get_bit_range (unsigned HOST_WIDE_INT *b
*** 4544,4549 ****
--- 4544,4570 ----
}
}
+ /* Return the alignment of the object EXP, also considering its type
+ when we do not know of explicit misalignment.
+ ??? 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 the alignment of the
+ type when we cannot compute a misalignment. */
+
+ static unsigned int
+ get_object_or_type_alignment (tree exp)
+ {
+ unsigned HOST_WIDE_INT misalign;
+ unsigned int align = get_object_alignment_1 (exp, &misalign);
+ if (misalign != 0)
+ align = (misalign & -misalign);
+ else
+ align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), align);
+ return align;
+ }
+
/* Expand an assignment that stores the value of FROM into TO. If NONTEMPORAL
is true, try generating a nontemporal store. */
*************** expand_assignment (tree to, tree from, b
*** 4553,4559 ****
rtx to_rtx = 0;
rtx result;
enum machine_mode mode;
! int align;
enum insn_code icode;
/* Don't crash if the lhs of the assignment was erroneous. */
--- 4574,4580 ----
rtx to_rtx = 0;
rtx result;
enum machine_mode mode;
! unsigned int align;
enum insn_code icode;
/* Don't crash if the lhs of the assignment was erroneous. */
*************** expand_assignment (tree to, tree from, b
*** 4571,4578 ****
if ((TREE_CODE (to) == MEM_REF
|| TREE_CODE (to) == TARGET_MEM_REF)
&& mode != BLKmode
! && ((align = MAX (TYPE_ALIGN (TREE_TYPE (to)), get_object_alignment
(to)))
! < (signed) GET_MODE_ALIGNMENT (mode))
&& ((icode = optab_handler (movmisalign_optab, mode))
!= CODE_FOR_nothing))
{
--- 4592,4599 ----
if ((TREE_CODE (to) == MEM_REF
|| TREE_CODE (to) == TARGET_MEM_REF)
&& mode != BLKmode
! && ((align = get_object_or_type_alignment (to))
! < GET_MODE_ALIGNMENT (mode))
&& ((icode = optab_handler (movmisalign_optab, mode))
!= CODE_FOR_nothing))
{
*************** expand_expr_real_1 (tree exp, rtx target
*** 9241,9247 ****
addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (exp));
struct mem_address addr;
enum insn_code icode;
! int align;
get_address_description (exp, &addr);
op0 = addr_for_mem_ref (&addr, as, true);
--- 9262,9268 ----
addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (exp));
struct mem_address addr;
enum insn_code icode;
! unsigned int align;
get_address_description (exp, &addr);
op0 = addr_for_mem_ref (&addr, as, true);
*************** expand_expr_real_1 (tree exp, rtx target
*** 9249,9257 ****
temp = gen_rtx_MEM (mode, op0);
set_mem_attributes (temp, exp, 0);
set_mem_addr_space (temp, as);
! align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), get_object_alignment (exp));
if (mode != BLKmode
! && (unsigned) align < GET_MODE_ALIGNMENT (mode)
/* If the target does not have special handling for unaligned
loads of mode then it can use regular moves for them. */
&& ((icode = optab_handler (movmisalign_optab, mode))
--- 9270,9278 ----
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 (mode != BLKmode
! && align < GET_MODE_ALIGNMENT (mode)
/* If the target does not have special handling for unaligned
loads of mode then it can use regular moves for them. */
&& ((icode = optab_handler (movmisalign_optab, mode))
*************** expand_expr_real_1 (tree exp, rtx target
*** 9278,9284 ****
tree base = TREE_OPERAND (exp, 0);
gimple def_stmt;
enum insn_code icode;
! int align;
/* Handle expansion of non-aliased memory with non-BLKmode. That
might end up in a register. */
if (TREE_CODE (base) == ADDR_EXPR)
--- 9299,9305 ----
tree base = TREE_OPERAND (exp, 0);
gimple def_stmt;
enum insn_code icode;
! unsigned align;
/* Handle expansion of non-aliased memory with non-BLKmode. That
might end up in a register. */
if (TREE_CODE (base) == ADDR_EXPR)
*************** expand_expr_real_1 (tree exp, rtx target
*** 9329,9335 ****
gimple_assign_rhs1 (def_stmt), mask);
TREE_OPERAND (exp, 0) = base;
}
! align = MAX (TYPE_ALIGN (TREE_TYPE (exp)), 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)))
--- 9350,9356 ----
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)))
*************** expand_expr_real_1 (tree exp, rtx target
*** 9345,9351 ****
if (TREE_THIS_VOLATILE (exp))
MEM_VOLATILE_P (temp) = 1;
if (mode != BLKmode
! && (unsigned) align < GET_MODE_ALIGNMENT (mode)
/* If the target does not have special handling for unaligned
loads of mode then it can use regular moves for them. */
&& ((icode = optab_handler (movmisalign_optab, mode))
--- 9366,9372 ----
if (TREE_THIS_VOLATILE (exp))
MEM_VOLATILE_P (temp) = 1;
if (mode != BLKmode
! && align < GET_MODE_ALIGNMENT (mode)
/* If the target does not have special handling for unaligned
loads of mode then it can use regular moves for them. */
&& ((icode = optab_handler (movmisalign_optab, mode))
