Re: [PATCH] Cleanup of IPA-CP alignment lattices

[Martin Jambor]

Hi,

On Thu, Oct 08, 2015 at 12:08:15AM +0200, Jan Hubicka wrote:
> > > This patch broke Solaris bootstrap in stage 1 with g++ 4.9:
> > > 
> > > /vol/gcc/src/hg/trunk/solaris/gcc/ipa-cp.c: In member function 'bool 
> > > ipcp_alignment_lattice::meet_with_1(unsigned int, unsigned int)':
> > > /vol/gcc/src/hg/trunk/solaris/gcc/ipa-cp.c:855:56: error: call of 
> > > overloaded 'abs(unsigned int)' is ambiguous
> > >        int diff = abs (misalign - (new_misalign % align));
> > 
> > Calling abs on an unsigned type does not sound right anyway (that
> > almost sounds like a good candidate for a warning).  I suppose the
> 
> Yep, I guess that may be a good idea ;)
> 
> > correct fix is to cast both subtraction operands to signed int, would
> > such a change be pre-approved?
> 
>   if (misalign != (new_misalign % align))
>     {
>       int diff = abs (misalign - (new_misalign % align));
>       align = MIN (align, (unsigned) diff & -diff);
>       if (align)
>         misalign = misalign % align;
>       else
>         set_to_bottom ();
>       changed = true;
>     }
> 
> So the logic here is that you compute differnce of missaligns and want to set 
> align to be at least that.  Why
>       align = MIN (align, (unsigned) diff & -diff);
> I suppose ALIGN should be always greater than that, because diff is at most 
> ALIGN.
> So I suppose you want 
>   align = (unsigned) diff & -diff
> 
> The changes are pre-approved.
> 


Thanks, I have committed the following after bootstrapping and testing
on x86_64-linux.

Martin


2015-10-08  Martin Jambor  <mjam...@suse.cz>

        * ipa-cp.c (meet_with_1): Make the argument of abs signed.  Remove
        unnecessary MIN.

diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 0d9fdee..d9d81f1 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -852,8 +852,8 @@ ipcp_alignment_lattice::meet_with_1 (unsigned new_align, 
unsigned new_misalign)
     }
   if (misalign != (new_misalign % align))
     {
-      int diff = abs (misalign - (new_misalign % align));
-      align = MIN (align, (unsigned) diff & -diff);
+      int diff = abs ((int) misalign - (int) (new_misalign % align));
+      align = (unsigned) diff & -diff;
       if (align)
        misalign = misalign % align;
       else