[PATCH/RFC] PR other/22313: Hot/cold sections breaks dwarf2

[Roger Sayle <roger at eyesopen dot com>]

PR other/22313 is marked in bugzilla as a critical P1 regression where
"make profiledbootstrap" fails.  After much investigation, I can't get
this to fail on mainline, but can now reproduce the issue on the 4.1
branch when compiling for the i486.

The cause of the failure is the profile feedback causes us to split
a function into different text sections, which then creates problems
for asynchronous unwind tables in dwarf2, as it then takes the difference
between .LCFI* labels in different sections whilst emitting a
DW_CFA_advance_loc4 thingy.

As suggested by Dan Jacobowitz on IRC, if we really have CFI instruction
labels in different sections, we need to emit a DW_CFA_set_loc to update
the unwind table to the new location rather than attempt to update
incrementally by calculating an offset.

My first attempt at a patch, tried to reset fde->dw_fde_current_label
in dwarf2out_switch_text_section, and then tweak add_fde_cfi to emit
a set_loc if the previous label was NULL, or a "advance_loc4" if it
is in the same section.  Alas, this attempt failed, as it appears the
switch section calls are not interleaved with add_fde_cfi, but instead
occur as a separate pass.

The attempt below current the problem with a bigger hammer, of always
using "set_loc" in a function with hot/cold text sections, but continuing
to use "advance_loc4" for functions where this isn't required (i.e. by
default).

My first concern is why isn't this an issue on mainline.  Either some
fix for hot/cold vs. dwarf2 has already been applied, and needs to be
backported for 4.1, or another change has made the particular problem
latent.  Given this only affects i486 and not i686, is also bizarre,
but the different BRANCH_COSTs could result in different code, with
different edge frequencies causing different results in block
partitioning.

If the intention is to support both hot/cold block partitioning and
asynchronous unwind tables in a single function, then some variant of
the patch below is needed.  If anyone has any thoughts/insights on
how to tweak it such that we only use set_loc for jumping between
sections, and advance_loc4 the rest of the time, I suspect that would
be an improvement on the current scheme.  Unfortunately, this code
tracks label names as "const char*", so it's not a small change to
encode the section in the label representation.

Any thoughts?  Does the problem analysis ring any bells?


2006-07-23  Roger Sayle  <roger@eyesopen.com>

	PR other/22313
	* dwarf2out.c (add_fde_cfi): Use DW_CFA_set_loc instead of
	DW_CFA_advance_loc4 to update the location counter if we've
	switched text sections in this function.


Index: dwarf2out.c
===================================================================
*** dwarf2out.c	(revision 115677)
--- dwarf2out.c	(working copy)
*************** add_fde_cfi (const char *label, dw_cfi_r
*** 660,666 ****

  	  /* Set the location counter to the new label.  */
  	  xcfi = new_cfi ();
! 	  xcfi->dw_cfi_opc = DW_CFA_advance_loc4;
  	  xcfi->dw_cfi_oprnd1.dw_cfi_addr = label;
  	  add_cfi (&fde->dw_fde_cfi, xcfi);
  	}
--- 660,668 ----

  	  /* Set the location counter to the new label.  */
  	  xcfi = new_cfi ();
! 	  xcfi->dw_cfi_opc = fde->dw_fde_switched_sections
! 			     ? DW_CFA_set_loc
! 			     : DW_CFA_advance_loc4;
  	  xcfi->dw_cfi_oprnd1.dw_cfi_addr = label;
  	  add_cfi (&fde->dw_fde_cfi, xcfi);
  	}


Roger
--