Re: [RFC] Extend ipa-bitwise-cp with pointer alignment propagation

[Jan Hubicka <hubicka at ucw dot cz>]

> Hi,
> Sorry for late response, I was travelling.
> I tried to verify the alignments are monotonously worse with the
> attached patch (verify.diff),
> which asserts that alignment lattice is not better than bits lattice
> during each propagation
> step in propagate_constants_accross_call().
> Does that look OK ?

Yes, that looks fine to me.
> 
> ipa-cp-alignment has better alignments than ipa-bit-cp in following cases:
> 
> a) ipa_get_type() returns NULL: ipa-bits-cp sets lattice to bottom if
> ipa_get_type (param) returns NULL,
> for instance in case of K&R function, while ipa-cp-alignment doesn't
> look at param types,
> and can propagate alignments.
> The following assert:
> if (bits_lattice.bottom_p ())
>   gcc_assert (align_lattice.bottom_p())
> 
> triggered for 400.perlbench, 403.gcc, 456.hmmer and 481.wrf due to
> ipa_get_type()
> returning NULL. I am not really sure how to handle this case, since we
> need to know parameter's
> type during bits propagation for obtaining precision.

Indeed, it looks like a but in alignment propagation?
> 
> b) This happens for attached test-case (test.i),
> which is a reduced (and slightly modified) test-case from 458.sjeng.
> Bits propagation sets lattice to bottom, while alignment propagation
> propagates <align 1, misalign 0>.
> 
> In bits_lattice::meet_with_1
> m_mask = other_mask = 0x0fffffffffffffff0
> m_value = 0x7
> other_value = 0x8
> 
> In this case meet operation sets m_mask to:
> (m_mask | mask) | (m_value ^ other_value) = 0x0fffffffffffffff0 |
> (0xf) == 0x0ffffffffffffffff
> and hence the bits lattice is set to bottom.
> I suppose it doesn't matter for this case if bits propagation sets
> lattice to bottom,
> since propagating <align 1, misalign 0> isn't really useful ?

Hmm, Martin probably knows how the alignment is defined here.  If it is power of two,
then it would be alignment to even byte :)
> 
> The attached patch (alignprop-4.diff) removes ipa-cp-alignment, and
> checks for misalign against old_misalgin and prints message in the dump file
> if they mismatch. Testing in progress.

Looks OK to me (with Changelog please)
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 0e01577..601a347 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -1565,10 +1565,6 @@ fipa-cp-clone
>  Common Report Var(flag_ipa_cp_clone) Optimization
>  Perform cloning to make Interprocedural constant propagation stronger.
>  
> -fipa-cp-alignment
> -Common Report Var(flag_ipa_cp_alignment) Optimization
> -Perform alignment discovery and propagation to make Interprocedural constant propagation stronger.

I wonder if we should mark option as obsoleted and point users to consider -fno-ipa-bit-cp
instead.

Honza