On Tue, Oct 25, 2011 at 2:22 PM, Tom de Vries <[email protected]> wrote:
> On 09/24/2011 01:42 PM, Richard Guenther wrote:
>> On Sat, Sep 24, 2011 at 11:40 AM, Jakub Jelinek <[email protected]> wrote:
>>> On Sat, Sep 24, 2011 at 11:31:25AM +0200, Richard Guenther wrote:
>>>> In the end I'd probably say the patch is ok without the option (thus
>>>> turned on by default), but if LC_GLOBAL_LOCALE is part of the
>>>> glibc ABI then we clearly can't do this.
>>>
>>> Yes, LC_GLOBAL_LOCALE is part of glibc ABI. I guess we could only assume
>>> the alignment if the pointer is actually dereferenced on the statement
>>> that checks the ABI or in some stmt that dominates the spot where you want
>>> to check the alignment. It is IMHO quite common to pass arbitrary values
>>> in pointer types, then cast them back or just compare.
>>
>> Yeah (even if technically invoking undefined behavior in C). Checking if
>> there is a dereference post-dominating function entry with sth like
>>
>> FOR_EACH_IMM_USE_STMT (... ptr ...)
>> if (stmt_post_dominates_entry && contains derefrence of ptr)
>> alignment = TYPE_ALIGN (...);
>>
>> and otherwise not assuming anything about parameter alignment might work.
>> Be careful to check the alignment of the dereference though,
>>
>> typedef int int_unaligned __attribute__((aligned(1)));
>> int foo (int *p)
>> {
>> int_unaligned *q = p;
>> return *q;
>> }
>>
>> will be MEM[p] but with (well, hopefully ;)) TYPE_ALIGN of TREE_TYPE (MEM[p])
>> being 1. And yes, you'd have to look into handled-components as well. I
>> guess
>> you'll face similar problems as we do with tree-sra.c
>> tree_non_mode_aligned_mem_p
>> (you need to assume eventually misaligned accesses the same way expansion
>> does for the dereference, otherwise you'll run into issues on
>> strict-align targets).
>>
>> As that de-refrence thing doesn't really fit the CCP propagation you
>> won't be able
>> to handle
>>
>> int foo (int *p)
>> {
>> int *q = (char *)p + 3;
>> return *q;
>> }
>>
>> and assume q is aligned (and p is misaligned by 1).
>>
>> That is, if the definition of a pointer is post-dominated by a derefrence
>> we could assume proper alignment for that pointer (as opposed to just
>> special-casing its default definition). Would be certainly interesting to
>> see what kind of fallout we would get from that ;)
>>
>
> I gave this a try in deduce_alignment_from_dereferences.
>
> The fall-out I got from this were unaligned dereferenced pointers in
> gcc.c-torture/unsorted/*{cmp,set}.c.
>
> Bootstrapped and reg-tested on x86_64. Build and reg-tested on MIPS and ARM.
>
> Ok for trunk?
Can you not do the get_value_from_alignment split (it doesn't look
necessary to me) and drop the
@@ -541,10 +550,18 @@ get_value_for_expr (tree expr, bool for_
if (TREE_CODE (expr) == SSA_NAME)
{
val = *get_value (expr);
- if (for_bits_p
- && val.lattice_val == CONSTANT
+ if (!for_bits_p)
+ return val;
+
+ if (val.lattice_val == CONSTANT
&& TREE_CODE (val.value) == ADDR_EXPR)
val = get_value_from_alignment (val.value);
+ else if (val.lattice_val == VARYING
+ && SSA_NAME_PTR_INFO (expr) != NULL
+ && SSA_NAME_PTR_INFO (expr)->align > 1
+ && SSA_NAME_PTR_INFO (expr)->misalign == 0)
+ val = get_align_value (SSA_NAME_PTR_INFO (expr)->align * BITS_PER_UNIT,
+ TREE_TYPE (expr), 0);
}
hunk? I'm not sure why it is necessary at all - CCP is the only pass
computing alignment, so it should simply re-compute the info?
Anyway, it looks unrelated to the purpose of the patch in general.
The error reporting in deduce_alignment_from_dereferences is bogus,
the programs are undefined only at runtime, so you can at most
issue a warning.
+ /* Needs to be the successor of entry, for CDI_POST_DOMINATORS. */
+ entry = single_succ (ENTRY_BLOCK_PTR);
+
+ FOR_EACH_BB (bb)
+ {
+ gimple_stmt_iterator i;
+
+ if (!dominated_by_p (CDI_POST_DOMINATORS, entry, bb))
+ continue;
if you only consider post-dominators of the entry block then just walk
them directly (first_dom_son / next_dom_son).
+ align = TYPE_ALIGN (TREE_TYPE (memref)) / BITS_PER_UNIT;
+ if (align == 1)
+ continue;
I think you want to match what expand thinks of the alignment of this
memory reference, not just what TYPE_ALIGN says (and yes, that
needs to be split out somehow, SRA would need this as well).
+ while (TREE_CODE (ptr) == SSA_NAME)
+ {
+ pi = get_ptr_info (ptr);
+ if (pi->misalign != 0)
+ {
+ error ("misaligned pointer dereferenced");
+ break;
+ }
simply looking at pi->misalign is wrong. pi->align may be bigger
than the align that you computed above, so pi->misalign % align != 0
would be the right check.
+ if (pi->align >= align)
+ break;
+ pi->align = align;
and then set pi->misalign to zero here. But I would initialize the
CCP lattice with this, not set SSA_NAME_PTR_INFO, from
ccp_initialize, that already walks over all blocks and statements.
+ if (gimple_assign_rhs_code (def) == POINTER_PLUS_EXPR)
+ {
+ offset = gimple_assign_rhs2 (def);
+ if (!host_integerp (offset, 0)
+ || tree_low_cst (offset, 0) % align != 0)
+ break;
+
+ ptr = gimple_assign_rhs1 (def);
properly tracking explicit misalignment would be useful here, but I
see you are posting a proof-of-concept?
/* Main entry point for SSA Conditional Constant Propagation. */
static unsigned int
do_ssa_ccp (void)
{
+ calculate_dominance_info (CDI_POST_DOMINATORS);
+ deduce_alignment_from_dereferences ();
you need to free post-dom info again.
The
* gcc.c-torture/unsorted/HIcmp.c: Fix unaligned pointer.
* gcc.c-torture/unsorted/HIset.c: Same.
* gcc.c-torture/unsorted/SIcmp.c: Same.
* gcc.c-torture/unsorted/SIset.c: Same.
* gcc.c-torture/unsorted/SFset.c: Same.
* gcc.c-torture/unsorted/UHIcmp.c: Same.
* gcc.c-torture/unsorted/USIcmp.c: Same.
* gcc.c-torture/unsorted/DFcmp.c: Same.
changes are ok with s/sizeof/alignof/
Thanks,
Richard.
> Thanks,
> - Tom
>
>> Richard.
>>
>>> Jakub
>>>
>
> 2011-10-25 Tom de Vries <[email protected]>
>
> PR target/43814
> * tree-ssa-ccp.c (get_align_value): New function, factored out of
> get_value_from_alignment.
> (get_value_from_alignment): Use get_align_value.
> (get_value_for_expr): Use get_align_value to handle alignment of
> pointers with ptr_info->align set.
> (deduce_alignment_from_dereferences): New function.
> (do_ssa_ccp): Calculate post-dominance info and call
> deduce_alignment_from_dereferences.
>
> * gcc/testsuite/gcc.target/arm/pr43814-2.c: New test.
> * gcc.c-torture/unsorted/HIcmp.c: Fix unaligned pointer.
> * gcc.c-torture/unsorted/HIset.c: Same.
> * gcc.c-torture/unsorted/SIcmp.c: Same.
> * gcc.c-torture/unsorted/SIset.c: Same.
> * gcc.c-torture/unsorted/SFset.c: Same.
> * gcc.c-torture/unsorted/UHIcmp.c: Same.
> * gcc.c-torture/unsorted/USIcmp.c: Same.
> * gcc.c-torture/unsorted/DFcmp.c: Same.
>
