On Thu, Sep 22, 2016 at 11:21 AM, Richard Biener
<richard.guent...@gmail.com> wrote:
> On Wed, Sep 21, 2016 at 6:44 PM, Prathamesh Kulkarni
> <prathamesh.kulka...@linaro.org> wrote:
>> Hi,
>> The attached patch tries to extend ipa bits propagation to handle
>> pointer alignment propagation.
>> The patch just disables ipa-cp-alignment pass, I suppose we want to
>> eventually remove it ?
>>
>> Bootstrap+tested on x86_64-unknown-linux-gnu.
>> Cross-tested on arm*-*-*, aarch64*-*-*.
>> Does the patch look OK ?
>
> Just looking at the alignment extraction:
>
> + else
>
> if (POINTER_TYPE_P (...))
>
> + {
> + unsigned tem = bits[i].mask.to_uhwi ();
> + unsigned HOST_WIDE_INT bitpos = bits[i].value.to_uhwi ();
> + unsigned align = tem & -tem;
> + unsigned misalign = bitpos & (align - 1);
> ...
> + if (old_known
> + && old_align > align)
> + {
> + if (dump_file)
> + fprintf (dump_file, "But alignment was already %u.\n",
> old_align);
> + continue;
> + }
>
> it would be nice to sanity check old misalign against misalign.
> Basically
>
> gcc_assert (misalign & (old_align - 1) == old_misalign)
>
> here (and in the old_align > align case the reverse).
Just occured to me that this might trigger on invalid user code where correct
misalignment gets propagated in IPA but local bogus alignment doesn't
cause an issue because we're on a ! strict-align target. So maybe instead
of asserting output a warning here ... or at least put sth in the dumpfile.
> + set_ptr_info_alignment (pi, align, misalign);
>
>
> - ret |= propagate_alignment_accross_jump_function (cs, jump_func,
> -
> &dest_plats->alignment);
> +// ret |= propagate_alignment_accross_jump_function (cs, jump_func,
> +//
>
> this should of course be removed rather than commented.
>
> Leaving the IPA parts to somebody else.
>
> Richard.
>
>
>> Thanks,
>> Prathamesh
