This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH: Choose a better GP for ia64


On Wed, Mar 08, 2006 at 12:07:08PM -0800, James E Wilson wrote:
> 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;

I think it should be

   if (max_vma - min_vma < 0x400000
       && ((max_vma - gp_val >= 0x200000)
 	  || (gp_val - min_vma > 0x200000)))
     gp_val = min_vma + 0x200000;

Otherwise, max_vma == 0x400000, min_vma == 0x100000 and gp_val ==
0x200000 won't cover the whole image.

> 
> 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.

Here is the updated patch.

Thanks.


H.J.
----
2006-03-08  H.J. Lu  <hongjiu.lu@intel.com>

	* elfxx-ia64.c (elfNN_ia64_choose_gp): Properly choose gp.

--- bfd/elfxx-ia64.c.gp	2006-03-08 12:40:15.000000000 -0800
+++ bfd/elfxx-ia64.c	2006-03-08 12:47:37.000000000 -0800
@@ -3928,14 +3928,16 @@ elfNN_ia64_choose_gp (abfd, info)
 	gp_val = got_sec->output_section->vma;
       else if (max_short_vma != 0)
 	gp_val = min_short_vma;
-      else
+      else if (max_vma - min_vma < 0x200000)
 	gp_val = min_vma;
+      else
+	gp_val = max_vma - 0x200000 + 8;
 
       /* If it is possible to address the entire image, but we
 	 don't with the choice above, adjust.  */
       if (max_vma - min_vma < 0x400000
-	  && max_vma - gp_val <= 0x200000
-	  && gp_val - min_vma > 0x200000)
+	  && (max_vma - gp_val >= 0x200000
+	      || gp_val - min_vma > 0x200000))
 	gp_val = min_vma + 0x200000;
       else if (max_short_vma != 0)
 	{


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]