RFA: Generate normal DWARF DW_LOC descriptors for non integer mode pointers

RFA: Generate normal DWARF DW_LOC descriptors for non integer mode pointers

[Nick Clifton]

Hi Guys,

  Currently dwarf2out.c:mem_loc_descriptor() has some special case
  code to handle the situation where an address is held in a register
  whose mode is not of type MODE_INT.  It generates a
  DW_OP_GNU_regval_type expression which may later on be converted into
  a frame pointer based expression.  This is a problem for targets which
  use a partial integer mode for their pointers (eg the msp430).  In
  such cases the conversion to a frame pointer based expression could
  be wrong if the frame pointer is not being used.

  For example the GDB testfile gdb/testsuite/gdb.base/advance.c contains
  this code fragment:

    int
    main ()
    {
      int result;
      int b, c;
      c = 5;
      b = 3;    /* advance this location */
    
      func (c); /* stop here after leaving current frame */

  which compiles to these instructions:
  
	suba	#6,	r1
	mov	#5,	4(r1)	
	mov	#3,	2(r1)	
	mov	4(r1),	r12	
	calla	#0		;<func>

  (Note that only r1 - the stack pointer - is used.  r4 - the frame
  pointer - is not).

  The debug information produced for the "c" local variable looks like
  this:

    Abbrev Number: 3 (DW_TAG_variable)
     DW_AT_name        : c
     DW_AT_decl_file   : 1
     DW_AT_decl_line   : 40
     DW_AT_type        : <0x37>
     DW_AT_location    : 5 byte block: f5 4 21 32 1c 	(DW_OP_GNU_regval_type: 4 (r4) <0x21>; DW_OP_lit2; DW_OP_minus)

  ie it says that "c" is stored in memory location "r4 - 2", which is
  wrong since register r4 is not even used in this function.

  The patch below addresses this problem by allowing the normal,
  register based descriptor to be produced when the mode is Pmode.

  With this patch applied the unexpected failure count in the GDB
  testsuite for the MSP430's -mlarge multilib changes from 2253 to 367.
  There are no regressions, for MSP430 or x86_64, and no changes to
  the GCC testsuite results for either target.

  OK to apply ?

Cheers
  Nick

gcc/ChangeLog
2016-05-16  Nick Clifton  <nickc@redhat.com>

	* dwarf2out.c (mem_loc_descriptor): Convert REG based addresses
	whose mode is Pmode into basereg descriptors even if Pmode is
        not an integer mode.

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c	(revision 236283)
+++ gcc/dwarf2out.c	(working copy)
@@ -13396,7 +13396,11 @@
       break;
 
     case REG:
-      if (GET_MODE_CLASS (mode) != MODE_INT
+      if ((GET_MODE_CLASS (mode) != MODE_INT
+	  /* Targets which have pointers that use a partial integer mode
+	     (eg the msp430x) still want their debug information to be
+	     based on the normal DWARF base register notation.  */
+	   && mode != Pmode)
 	  || (GET_MODE_SIZE (mode) > DWARF2_ADDR_SIZE
 	      && rtl != arg_pointer_rtx
 	      && rtl != frame_pointer_rtx

Re: RFA: Generate normal DWARF DW_LOC descriptors for non integer mode pointers

[Jeff Law]

On 05/16/2016 08:12 AM, Nick Clifton wrote:
> Hi Guys,
>
>   Currently dwarf2out.c:mem_loc_descriptor() has some special case
>   code to handle the situation where an address is held in a register
>   whose mode is not of type MODE_INT.  It generates a
>   DW_OP_GNU_regval_type expression which may later on be converted into
>   a frame pointer based expression.  This is a problem for targets which
>   use a partial integer mode for their pointers (eg the msp430).  In
>   such cases the conversion to a frame pointer based expression could
>   be wrong if the frame pointer is not being used.
>
>   For example the GDB testfile gdb/testsuite/gdb.base/advance.c contains
>   this code fragment:
>
>     int
>     main ()
>     {
>       int result;
>       int b, c;
>       c = 5;
>       b = 3;    /* advance this location */
>
>       func (c); /* stop here after leaving current frame */
>
>   which compiles to these instructions:
>
> 	suba	#6,	r1
> 	mov	#5,	4(r1)	
> 	mov	#3,	2(r1)	
> 	mov	4(r1),	r12	
> 	calla	#0		;<func>
>
>   (Note that only r1 - the stack pointer - is used.  r4 - the frame
>   pointer - is not).
>
>   The debug information produced for the "c" local variable looks like
>   this:
>
>     Abbrev Number: 3 (DW_TAG_variable)
>      DW_AT_name        : c
>      DW_AT_decl_file   : 1
>      DW_AT_decl_line   : 40
>      DW_AT_type        : <0x37>
>      DW_AT_location    : 5 byte block: f5 4 21 32 1c 	(DW_OP_GNU_regval_type: 4 (r4) <0x21>; DW_OP_lit2; DW_OP_minus)
>
>   ie it says that "c" is stored in memory location "r4 - 2", which is
>   wrong since register r4 is not even used in this function.
>
>   The patch below addresses this problem by allowing the normal,
>   register based descriptor to be produced when the mode is Pmode.
>
>   With this patch applied the unexpected failure count in the GDB
>   testsuite for the MSP430's -mlarge multilib changes from 2253 to 367.
>   There are no regressions, for MSP430 or x86_64, and no changes to
>   the GCC testsuite results for either target.
>
>   OK to apply ?
>
> Cheers
>   Nick
>
> gcc/ChangeLog
> 2016-05-16  Nick Clifton  <nickc@redhat.com>
>
> 	* dwarf2out.c (mem_loc_descriptor): Convert REG based addresses
> 	whose mode is Pmode into basereg descriptors even if Pmode is
>         not an integer mode.
I'm not real familiar with dwarf, so if one of other maintainers steps 
in and says this is OK, then ignore my comments/questions.

I may be missing something, but isn't it the transition to an FP 
relative address rather than a SP relative address that's the problem 
here?  Where does that happen?  Is it possible we've got the wrong 
DECL_RTL or somesuch?

Jeff

Re: RFA: Generate normal DWARF DW_LOC descriptors for non integer mode pointers

[Nick Clifton]

Hi Jeff,

>>   Currently dwarf2out.c:mem_loc_descriptor() has some special case
>>   code to handle the situation where an address is held in a register
>>   whose mode is not of type MODE_INT.  It generates a
>>   DW_OP_GNU_regval_type expression which may later on be converted into
>>   a frame pointer based expression.  This is a problem for targets which
>>   use a partial integer mode for their pointers (eg the msp430).  In
>>   such cases the conversion to a frame pointer based expression could
>>   be wrong if the frame pointer is not being used.

> I may be missing something, but isn't it the transition to an FP 
> relative address rather than a SP relative address that's the problem 
> here?

Yes, I believe so.

> Where does that happen?  

I did not track it down.  But whilst I was searching for the cause I came
across the code that is modified by the patch.  Reading the code it seemed
obvious to me that the special case for handling non INT_MODE register modes
was not intended for pointers, and when I tried out a small patch it worked.

> Is it possible we've got the wrong DECL_RTL or somesuch?

I don't think so.  I am not familiar with this code myself, but the dump from 
the dwarf2 pass shows:

  (insn 5 2 6 (set (mem/c:HI (plus:PSI (reg/f:PSI 1 R1)
                (const_int 4 [0x4])) [1 c+0 S2 A16])
        (const_int 5 [0x5])) /work/sources/binutils/current/gdb/testsuite/gdb.base/advance.c:41 12 {movhi}
     (nil))

which to me pretty clearly shows that "c" is being stored at R1+4.

Cheers
  Nick

Re: RFA: Generate normal DWARF DW_LOC descriptors for non integer mode pointers

[Nick Clifton]

[-- Attachment #1: Type: text/plain, Size: 2344 bytes --]

Hi Jeff,

>>> I may be missing something, but isn't it the transition to an FP
>>> relative address rather than a SP relative address that's the problem
>>> here?
>>
>> Yes, I believe so.
>>
>>> Where does that happen?

I think that it happens in dwarf2out.c:based_loc_descr()  which
detects the use of the frame pointer and works out that it is going 
to be eliminated to the stack pointer:

  /* We only use "frame base" when we're sure we're talking about the
     post-prologue local stack frame.  We do this by *not* running
     register elimination until this point, and recognizing the special
     argument pointer and soft frame pointer rtx's.  */
  if (reg == arg_pointer_rtx || reg == frame_pointer_rtx)
    {
      rtx elim = (ira_use_lra_p
		  ? lra_eliminate_regs (reg, VOIDmode, NULL_RTX)
		  : eliminate_regs (reg, VOIDmode, NULL_RTX));

      if (elim != reg)
        .....

The problem, I believe, is that based_loc_descr() is only called
from mem_loc_descriptor when the mode of the rtl concerned is an
MODE_INT.  For example:

    case REG:
      if (GET_MODE_CLASS (mode) != MODE_INT
	 [...]
      else
      if (REGNO (rtl) < FIRST_PSEUDO_REGISTER)
	mem_loc_result = based_loc_descr (rtl, 0, VAR_INIT_STATUS_INITIALIZED);

or, (this is another one that I found whilst investigating this 
problem further):

  case PLUS:
    plus:
      if (is_based_loc (rtl)
	  && (GET_MODE_SIZE (mode) <= DWARF2_ADDR_SIZE
	      || XEXP (rtl, 0) == arg_pointer_rtx
	      || XEXP (rtl, 0) == frame_pointer_rtx)
	  && GET_MODE_CLASS (mode) == MODE_INT)
	mem_loc_result = based_loc_descr (XEXP (rtl, 0),
					  INTVAL (XEXP (rtl, 1)),
					  VAR_INIT_STATUS_INITIALIZED);
      else


There are quite a few places in mem_loc_descriptor where the code checks
for the mode being in the MODE_INT class.  I am not exactly sure why.  I
think that it might be that the programmer thought that any expression that
does not involve integer based arithmetic cannot be expressed in DWARF CFA
notation and so would have to be handled specially.  If I am correct,
then it seems to me that the proper fix would be to use SCALAR_INT_MODE_P
instead.

I tried out the extended patch (attached) and it gave even better GDB 
results for the MSP430 and still no regressions (GCC or GDB) for MSP430 or
x86_64.

Is this enough justification ?

Cheers
  Nick


[-- Attachment #2: dwarf2out.c.patch.2 --]
[-- Type: application/x-troff-man, Size: 10502 bytes --]

Re: RFA: Generate normal DWARF DW_LOC descriptors for non integer mode pointers

[Jeff Law]

On 05/26/2016 10:16 AM, Nick Clifton wrote:
> Hi Jeff,
>
>>>> I may be missing something, but isn't it the transition to an FP
>>>> relative address rather than a SP relative address that's the problem
>>>> here?
>>>
>>> Yes, I believe so.
>>>
>>>> Where does that happen?
>
> I think that it happens in dwarf2out.c:based_loc_descr()  which
> detects the use of the frame pointer and works out that it is going
> to be eliminated to the stack pointer:
>
>   /* We only use "frame base" when we're sure we're talking about the
>      post-prologue local stack frame.  We do this by *not* running
>      register elimination until this point, and recognizing the special
>      argument pointer and soft frame pointer rtx's.  */
>   if (reg == arg_pointer_rtx || reg == frame_pointer_rtx)
>     {
>       rtx elim = (ira_use_lra_p
> 		  ? lra_eliminate_regs (reg, VOIDmode, NULL_RTX)
> 		  : eliminate_regs (reg, VOIDmode, NULL_RTX));
>
>       if (elim != reg)
>         .....
>
> The problem, I believe, is that based_loc_descr() is only called
> from mem_loc_descriptor when the mode of the rtl concerned is an
> MODE_INT.  For example:
>
>     case REG:
>       if (GET_MODE_CLASS (mode) != MODE_INT
> 	 [...]
>       else
>       if (REGNO (rtl) < FIRST_PSEUDO_REGISTER)
> 	mem_loc_result = based_loc_descr (rtl, 0, VAR_INIT_STATUS_INITIALIZED);
>
> or, (this is another one that I found whilst investigating this
> problem further):
>
>   case PLUS:
>     plus:
>       if (is_based_loc (rtl)
> 	  && (GET_MODE_SIZE (mode) <= DWARF2_ADDR_SIZE
> 	      || XEXP (rtl, 0) == arg_pointer_rtx
> 	      || XEXP (rtl, 0) == frame_pointer_rtx)
> 	  && GET_MODE_CLASS (mode) == MODE_INT)
> 	mem_loc_result = based_loc_descr (XEXP (rtl, 0),
> 					  INTVAL (XEXP (rtl, 1)),
> 					  VAR_INIT_STATUS_INITIALIZED);
>       else
>
>
> There are quite a few places in mem_loc_descriptor where the code checks
> for the mode being in the MODE_INT class.  I am not exactly sure why.  I
> think that it might be that the programmer thought that any expression that
> does not involve integer based arithmetic cannot be expressed in DWARF CFA
> notation and so would have to be handled specially.  If I am correct,
> then it seems to me that the proper fix would be to use SCALAR_INT_MODE_P
> instead.
>
> I tried out the extended patch (attached) and it gave even better GDB
> results for the MSP430 and still no regressions (GCC or GDB) for MSP430 or
> x86_64.
>
> Is this enough justification ?
So the argument is that MODE_INT was likely intended to filter out 
things like FP modes and such that might somehow be bogusly used as 
addresses.  As written those tests are also filtering out partial 
integer modes which we can support?

And that many (most?) of the things that filter on MODE_INT should 
really be using MODE_INT || MODE_PARTIAL_INT (aka SCALAR_INT_MODE_P).

I can buy that ;-)   OK with a suitable ChangeLog entry.

jeff

Re: RFA: Generate normal DWARF DW_LOC descriptors for non integer mode pointers

[Nick Clifton]

Hi Jeff,

> I can buy that ;-)   OK with a suitable ChangeLog entry.

Thanks!  Checked in with this changelog entry.

Cheers
  Nick

gcc/ChangeLog
2016-06-22  Nick Clifton  <nickc@redhat.com>

	* dwarf2out.c (scompare_loc_descriptor): Use SCALAR_INT_MODE_P() in
	place of GET_MODE_CLASS() == MODE_INT, so that partial integer
	modes are accepted as well.
	(ucompare_loc_descriptor): Likewise.
	(minmax_loc_descriptor): Likewise.
	(clz_loc_descriptor): Likewise.
	(popcount_loc_descriptor): Likewise.
	(bswap_loc_descriptor): Likewise.
	(rotate_loc_descriptor): Likewise.
	(mem_loc_descriptor): Likewise.
	(loc_descriptor): Likewise.