> > +/* Decrease alignment info DEST to be at most CUR.  */
> > +
> > +static bool
> > +decrease_alignment (ipa_alignment *dest, ipa_alignment cur)
> > +{
> > +  bool changed = false;
> > +
> > +  if (!cur.known)
> > +    return false;
> 
> I really think this should be return set_alignment_to_bottom (dest);
> 
> If some known alignment has been already propagated to DEST along a
> different edge and now along the current edge an unknown alignment is
> coming in, then the result value of the lattice must be BOTTOM and not
> the previous alignment this code leaves in place.

Well, because this is an optimisitic propagation now, !cur.known means TOP
that is "as good alginment as you can thunk of".
You have one known alignment in DEST and TOP in other, result is TOP.
> > +/* Increase alignment info DEST to be at least CUR.  */
> > +
> > +static bool
> > +increase_alignment (ipa_alignment *dest, ipa_alignment cur)
> > +{
> > +  if (!dest->known)
> > +    return false;
> 
> I think this condition always exits.  DEST is caller's CUR which was
> read from jfunc->alignment which is always going to be unknown for
> PASS_THROUGH and ANCESTOR jump functions at the moment.  Perhaps you
> meant if (!cur.known) ?

Again, it is just lattice operation. If dest is TOP, there is no
way to get it better.

I assumed that jfunc->alignment.known is meaningful even for PASSTHROUGH
(for example for parameters passed by invisible reference) and I turn
all !jfunc->alignment.known to BOTTOM in the conditional at beggining of
propagate_alignment_accross_jump_function.
If we never have useful alginment info on passthrough and ancestor,
I suppose we can just drop increase_alignment for now and always
go with CUR.
> 
> > +  if (!cur.known || dest->align < cur.align)
> > +    {
> 
> again here, I think you meant !des->.known.

Well, if CUR is TOP, you want to return CUR :)

Honza
> 
> Apart from that, I am fine with the patch.
> Thanks,
> 
> Martin

Reply via email to