Re: PATCH: Choose a better GP for ia64

[James E Wilson <wilson at specifix dot com>]

On Wed, 2006-03-08 at 10:20, H. J. Lu wrote:
> 2. I don't quite understand what this code
>       if (max_vma - min_vma < 0x400000
>           && max_vma - gp_val <= 0x200000
>           && gp_val - min_vma > 0x200000)
>         gp_val = min_vma + 0x200000;
> is trying to do.

This handles the case where the gp_val is set too high.  Suppose we end
up with max_vma == 0x400000, gp_val == 0x400000, and min_vma ==
0x100000.  This code will trigger, and set gp_val to 0x300000.  Your
revised code won't handle this case.

However, you are right that the current code won't handle the case where
gp_val was set too low.  So, really, there are two cases we need to test
for and handle here.  One case where gp_val was set too high, and one
case where gp_val was set too low.  So I think the code should be
something like
  if (max_vma - min_vma < 0x400000
      && ((max_vma - gp_val > 0x200000)
	  || (gp_val - min_vma > 0x200000)))
    gp_val = min_vma + 0x200000;

Also, this code
+      else if (max_vma - min_vma < 0x400000)
        gp_val = min_vma;
I think should be testing against 0x200000 instead of 0x400000. 
Otherwise, you are giving gp_val a value which may be wrong, since it
won't cover the entire image if it is between 0x200000 and 0x400000. 
This also makes the code easier to understand, since clearly the next
line is not safe if the image size is less than 0x200000.
-- 
Jim Wilson, GNU Tools Support, http://www.specifix.com