Re: PATCH: Fix handle implicit nops for unwind records

[Jim Wilson <wilson at specifixinc dot com>]

On Fri, 2004-01-16 at 14:52, H. J. Lu wrote:
> On Thu, Jan 15, 2004 at 12:13:11PM -0800, David Mosberger wrote:
> > One thing it still doesn't seem to get right is if there are implicit
> > nops at the beginning of a region.  For example:

I finally got around to trying my suggestion.  It ended up a little more
complicated than I had hoped, but it seems to be working well.  This
patch works for the testcase David provided.  I also tested it against
the Linux kernel.  Quite a few prologue regions get one or two insns
shorter, because they no longer contain following nops.  Also, quite a
few body regions that contain epilogue records get one of two insns
shorter, which reduces the epilogue count.  These seem like desirable
changes.  There are no other changes to the unwind info.

While doing this, I noticed that padding unwind records used to be all
zeros (R1:prologue(rlen=0)) but now we get occasional non-zero bytes in
there.  This gives weird looking results when you dump the unwind
records.  This is a side-effect of HJ's patch to use a variant frag for
the unwind info.  I added a patch to fix that too.

I am also wondering if there is a problem with the personality routine. 
I think we can get garbage there also when it is not set, but I don't
have a testcase for that, as readelf -u does not print the personality
routine when it isn't being used.  I also noticed that we seem to have
overlapping frags when the personality routine is present.  The
personality routine is a separate frag, but it is also included in the
size of the variant frag.  I don't know if that is kosher, but it looks
funny to me.  This doesn't cause an immediate problem because we don't
write to that part of the variant frag.

It being 2AM, I will wait until tomorrow to check it in.  It also just
occurred to me that I forgot to run the binutils testsuite.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com

2004-01-31  James E Wilson  <wilson@specifixinc.com>

	* config/tc-ia64.c (ia64_convert_frag): Call md_number_to_chars to
	fill padding bytes with zeroes.
	(emit_one_bundle): New locals last_ptr, end_ptr.  Rewrite code that
	sets unwind_record slot_number and slot_frag fields.

Index: tc-ia64.c
===================================================================
RCS file: /cvs/src/src/gas/config/tc-ia64.c,v
retrieving revision 1.97
diff -p -r1.97 tc-ia64.c
*** tc-ia64.c	28 Jan 2004 23:00:02 -0000	1.97
--- tc-ia64.c	31 Jan 2004 08:52:10 -0000
*************** ia64_convert_frag (fragS *frag)
*** 2911,2916 ****
--- 2911,2922 ----
    /* Skip the header.  */
    vbyte_mem_ptr = frag->fr_literal + 8;
    process_unw_records (list, output_vbyte_mem);
+ 
+   /* Fill the padding bytes with zeros.  */
+   if (pad != 0)
+     md_number_to_chars (frag->fr_literal + len + 8 - md.pointer_size + pad, 0,
+ 			md.pointer_size - pad);
+ 
    frag->fr_fix += size;
    frag->fr_type = rs_fill;
    frag->fr_var = 0;
*************** emit_one_bundle ()
*** 6032,6038 ****
    struct ia64_opcode *idesc;
    int end_of_insn_group = 0, user_template = -1;
    int n, i, j, first, curr;
!   unw_rec_list *ptr;
    bfd_vma t0 = 0, t1 = 0;
    struct label_fix *lfix;
    struct insn_fix *ifix;
--- 6038,6044 ----
    struct ia64_opcode *idesc;
    int end_of_insn_group = 0, user_template = -1;
    int n, i, j, first, curr;
!   unw_rec_list *ptr, *last_ptr, *end_ptr;
    bfd_vma t0 = 0, t1 = 0;
    struct label_fix *lfix;
    struct insn_fix *ifix;
*************** emit_one_bundle ()
*** 6076,6093 ****
    end_of_insn_group = 0;
    for (i = 0; i < 3 && md.num_slots_in_use > 0; ++i)
      {
!       /* Set the slot number for prologue/body records now as those
! 	 refer to the current point, not the point after the
! 	 instruction has been issued:  */
!       /* Don't try to delete prologue/body records here, as that will cause
! 	 them to also be deleted from the master list of unwind records.  */
!       for (ptr = md.slot[curr].unwind_record; ptr; ptr = ptr->next)
! 	if (ptr->r.type == prologue || ptr->r.type == prologue_gr
! 	    || ptr->r.type == body)
! 	  {
! 	    ptr->slot_number = (unsigned long) f + i;
! 	    ptr->slot_frag = frag_now;
! 	  }
  
        if (idesc->flags & IA64_OPCODE_SLOT2)
  	{
--- 6082,6120 ----
    end_of_insn_group = 0;
    for (i = 0; i < 3 && md.num_slots_in_use > 0; ++i)
      {
!       /* If we have unwind records, we may need to update some now.  */
!       ptr = md.slot[curr].unwind_record;
!       if (ptr)
! 	{
! 	  /* Find the last prologue/body record in the list for the current
! 	     insn, and set the slot number for all records up to that point.
! 	     This needs to be done now, because prologue/body records refer to
! 	     the current point, not the point after the instruction has been
! 	     issued.  This matters because there may have been nops emitted
! 	     meanwhile.  Any non-prologue non-body record followed by a
! 	     prologue/body record must also refer to the current point.  */
! 	  last_ptr = NULL;
! 	  end_ptr = md.slot[(curr + 1) % NUM_SLOTS].unwind_record;
! 	  for (; ptr != end_ptr; ptr = ptr->next)
! 	    if (ptr->r.type == prologue || ptr->r.type == prologue_gr
! 		|| ptr->r.type == body)
! 	      last_ptr = ptr;
! 	  if (last_ptr)
! 	    {
! 	      /* Make last_ptr point one after the last prologue/body
! 		 record.  */
! 	      last_ptr = last_ptr->next;
! 	      for (ptr = md.slot[curr].unwind_record; ptr != last_ptr;
! 		   ptr = ptr->next)
! 		{
! 		  ptr->slot_number = (unsigned long) f + i;
! 		  ptr->slot_frag = frag_now;
! 		}
! 	      /* Remove the initialized records, so that we won't accidentally
! 		 update them again if we insert a nop and continue.  */
! 	      md.slot[curr].unwind_record = last_ptr;
! 	    }
! 	}
  
        if (idesc->flags & IA64_OPCODE_SLOT2)
  	{
*************** emit_one_bundle ()
*** 6292,6306 ****
  
        build_insn (md.slot + curr, insn + i);
  
!       /* Set slot counts for non prologue/body unwind records.  */
!       for (ptr = md.slot[curr].unwind_record; ptr; ptr = ptr->next)
! 	if (ptr->r.type != prologue && ptr->r.type != prologue_gr
! 	    && ptr->r.type != body)
! 	  {
! 	    ptr->slot_number = (unsigned long) f + i;
! 	    ptr->slot_frag = frag_now;
! 	  }
!       md.slot[curr].unwind_record = NULL;
  
        if (required_unit == IA64_UNIT_L)
  	{
--- 6319,6338 ----
  
        build_insn (md.slot + curr, insn + i);
  
!       ptr = md.slot[curr].unwind_record;
!       if (ptr)
! 	{
! 	  /* Set slot numbers for all remaining unwind records belonging to the
! 	     current insn.  There can not be any prologue/body unwind records
! 	     here.  */
! 	  end_ptr = md.slot[(curr + 1) % NUM_SLOTS].unwind_record;
! 	  for (; ptr != end_ptr; ptr = ptr->next)
! 	    {
! 	      ptr->slot_number = (unsigned long) f + i;
! 	      ptr->slot_frag = frag_now;
! 	    }
! 	  md.slot[curr].unwind_record = NULL;
! 	}
  
        if (required_unit == IA64_UNIT_L)
  	{