[Bug debug/53948] New: [4.8 Regression] Assignment line missing for -O0 -g

[Bug debug/53948] New: [4.8 Regression] Assignment line missing for -O0 -g

[jan.kratochvil at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53948

             Bug #: 53948
           Summary: [4.8 Regression] Assignment line missing for -O0 -g
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: debug
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: jan.kratochvil@redhat.com
                CC: aoliva@gcc.gnu.org
            Target: x86_64-unknown-linux-gnu


Nothing serious but it broke gdb.base/store.exp and I believe -O0 -g should not
"optimize out" source statements.

int f (register int a, register int b) {
  register int x = b, y = a;
  return x + y; }
gcc -g

PASS: gcc (GCC) 4.7.2 20120713 (prerelease)
   7:    89 f8                    mov    %edi,%eax
   9:    89 f2                    mov    %esi,%edx
  register int x = b, y = a;
   b:    41 89 d4                 mov    %edx,%r12d
   e:    89 c3                    mov    %eax,%ebx
  return x + y; }
  10:    41 8d 04 1c              lea    (%r12,%rbx,1),%eax

FAIL: gcc (GCC) 4.8.0 20120713 (experimental)
   7:    89 fb                    mov    %edi,%ebx
   9:    41 89 f4                 mov    %esi,%r12d
  register int x = b, y = a;
  return x + y; }
   c:    41 8d 04 1c              lea    (%r12,%rbx,1),%eax

GDB no longer stops on the 'register int x = b, y = a;' line during 'step'.
.debug_info is correct in both cases.

I can update the GDB testsuite but still I believe 'int var = val;' source line
should have its instruction during -O0 -g, even if it is just a 'nop'.

[Bug debug/53948] [4.8 Regression] Assignment line missing for -O0 -g

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53948

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.8.0

[Bug debug/53948] [4.8 Regression] Assignment line missing for -O0 -g

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53948

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-07-16
                 CC|                            |jakub at gcc dot gnu.org
     Ever Confirmed|0                           |1

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-07-16 19:36:48 UTC ---
Caused by http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=189361
IMHO the ira part of the patch should be reverted.

[Bug debug/53948] [4.8 Regression] Assignment line missing for -O0 -g

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53948

Steven Bosscher <steven at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |steven at gcc dot gnu.org
         AssignedTo|unassigned at gcc dot       |steven at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #2 from Steven Bosscher <steven at gcc dot gnu.org> 2012-07-16 19:48:50 UTC ---
I don't see how reverting this helps. We should understand why REG_USERVAR_P is
apparently not true for this user variable.

[Bug debug/53948] [4.8 Regression] Assignment line missing for -O0 -g

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53948

--- Comment #3 from Steven Bosscher <steven at gcc dot gnu.org> 2012-07-16 20:23:20 UTC ---
The problem seems to be that PARM_DECLs are not marked as REG_USERVAR_P.

[Bug debug/53948] [4.8 Regression] Assignment line missing for -O0 -g

[steven at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53948

--- Comment #4 from Steven Bosscher <steven at gcc dot gnu.org> 2012-07-16 21:57:55 UTC ---
Created attachment 27806
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27806
Proposed fix: Distinguish between a user variable and a function argument

[Bug debug/53948] [4.8 Regression] Assignment line missing for -O0 -g

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53948

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug debug/53948] [4.8 Regression] Assignment line missing for -O0 -g

[law at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53948

Jeffrey A. Law <law at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |law at redhat dot com
         AssignedTo|steven at gcc dot gnu.org   |law at gcc dot gnu.org

--- Comment #5 from Jeffrey A. Law <law at redhat dot com> 2013-02-07 20:04:00 UTC ---
So, the real problem here is that replacing

VAR_OR_FUNCTION_DECL_P && ! DECL_ARTIFICAL

with REG_USERVAR_P

is not the same.  In particular the former is _false_ for parameters and the
latter is true.  c#3 is totally wrong, parameters are marked with
REG_USERVAR_P.

The real way to get the prior behaviour without reverting the patch is to
either explicitly mark parameters so we can check for them in this one hunk of
code.  Or to create a helper function in a suitable location that can map from
a reg to its decl, then check if it's a parameter.  Given that flags in the
main rtl structures are generally scarce, I think the latter is a better
solution given how rarely we need to make this distinction.

It's unfortunate that this P1 regression was left languishing, half analyzed in
our tree for 6+ months ;(

[Bug debug/53948] [4.8 Regression] Assignment line missing for -O0 -g

[stevenb.gcc at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53948

--- Comment #6 from stevenb.gcc at gmail dot com <stevenb.gcc at gmail dot com> 2013-02-07 20:24:07 UTC ---
On Thu, Feb 7, 2013 at 9:04 PM, law at redhat dot com wrote:
> The real way to get the prior behaviour without reverting the patch is to
> either explicitly mark parameters so we can check for them in this one hunk of
> code.

That is what I did in http://gcc.gnu.org/ml/gcc-patches/2012-07/msg00847.html

>  Or to create a helper function in a suitable location that can map from
> a reg to its decl, then check if it's a parameter.  Given that flags in the
> main rtl structures are generally scarce, I think the latter is a better
> solution given how rarely we need to make this distinction.

I used an rtl flag because I could make one available. Structures on
the side always make me nervous, they tend to be fragile and hard to
update.


> It's unfortunate that this P1 regression was left languishing, half analyzed in
> our tree for 6+ months ;(

Not true: http://gcc.gnu.org/ml/gcc-patches/2012-07/msg00847.html

It's unfortunate that the patch was never reviewed.

[Bug debug/53948] [4.8 Regression] Assignment line missing for -O0 -g

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53948

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-02-07 20:40:31 UTC ---
(In reply to comment #6)
> Not true: http://gcc.gnu.org/ml/gcc-patches/2012-07/msg00847.html

http://gcc.gnu.org/ml/gcc-patches/2012-12/msg01104.html ?

[Bug debug/53948] [4.8 Regression] Assignment line missing for -O0 -g

[law at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53948

--- Comment #8 from Jeffrey A. Law <law at redhat dot com> 2013-02-07 20:46:10 UTC ---
Also note the patch, to the best of my knowledge, was not ping'd.  As a
regression (and a P1 regression I might add) I would have expected Steven to
ping the patch and/or Jakub to have pushed harder for reversion of the original
patch which caused the regression per our patch reversion policy.

Either way it was not handled well. 


WRT on-the side datastructures.  The on-the-side structure here is the existing
tree structures.  At some point all the linkage between RTL structures and tree
structures needs to be sorted out, but I don't think now is the time nor do I
think the amount of usage here justifies using a precious bit in our base rtl
structure.

[Bug debug/53948] [4.8 Regression] Assignment line missing for -O0 -g

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53948

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> 2013-02-08 08:28:13 UTC ---
(In reply to comment #5)
> So, the real problem here is that replacing
> 
> VAR_OR_FUNCTION_DECL_P && ! DECL_ARTIFICAL
> 
> with REG_USERVAR_P
> 
> is not the same.  In particular the former is _false_ for parameters and the
> latter is true.  c#3 is totally wrong, parameters are marked with
> REG_USERVAR_P.
> 
> The real way to get the prior behaviour without reverting the patch is to
> either explicitly mark parameters so we can check for them in this one hunk of
> code.  Or to create a helper function in a suitable location that can map from
> a reg to its decl, then check if it's a parameter.

I thought this is what REG_EXPR provides ...

  Given that flags in the
> main rtl structures are generally scarce, I think the latter is a better
> solution given how rarely we need to make this distinction.
> 
> It's unfortunate that this P1 regression was left languishing, half analyzed in
> our tree for 6+ months ;(

[Bug debug/53948] [4.8 Regression] Assignment line missing for -O0 -g

[law at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53948

--- Comment #10 from Jeffrey A. Law <law at redhat dot com> 2013-02-08 13:19:46 UTC ---
Right, REG_EXPR gets the underlying decl, but without access to tree.h we can't
do anything useful with the decl.  The whole point of Steven's patch which
caused this regression was to avoid sucking in tree.h into ira-conflicts and
other backend files.

So the little helper just needs to live in expr.c or similar location where we
can peek at the contents of the decl.

The alternate is something like Steven's follow-up patch to create
REG_FUNCTION_PARM_P which gives us better separation at the expense of using
bits in the base rtl structure.

[Bug debug/53948] [4.8 Regression] Assignment line missing for -O0 -g

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53948

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-02-08 13:46:05 UTC ---
Yeah, I agree with Jeff here, IMHO a helper function somewhere where both rtl.h
and tree.h is included is best solution for now.
As long as REG/MEM have REG_EXPR/MEM_EXPR, there is no actual separation
between RTL IL and trees anyway, lots of RTL code calls the alias oracle that
looks at the trees referenced from the RTL expressions, so I don't see why
another little function couldn't be used instead of duplicating the info in the
scarce RTL flags.

[Bug debug/53948] [4.8 Regression] Assignment line missing for -O0 -g

[law at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53948

--- Comment #12 from Jeffrey A. Law <law at gcc dot gnu.org> 2013-02-08 20:04:04 UTC ---
Author: law
Date: Fri Feb  8 20:03:52 2013
New Revision: 195900

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195900
Log:
       PR debug/53948
        * emit-rtl.c (reg_is_parm_p): New function.
        * regs.h (reg_is_parm_p): New prototype.
        * ira-conflicts.c (ira_build_conflicts): Allow parameters in
        callee-clobbered registers.

       PR debug/53948
        * gcc.dg/debug/dwarf2/pr53948.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/debug/dwarf2/pr53948.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/emit-rtl.c
    trunk/gcc/ira-conflicts.c
    trunk/gcc/regs.h
    trunk/gcc/testsuite/ChangeLog

[Bug debug/53948] [4.8 Regression] Assignment line missing for -O0 -g

[law at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53948

--- Comment #13 from Jeffrey A. Law <law at redhat dot com> 2013-02-08 20:04:52 UTC ---
Patch installed on trunk

[Bug debug/53948] [4.8 Regression] Assignment line missing for -O0 -g

[law at redhat dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53948

Jeffrey A. Law <law at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #14 from Jeffrey A. Law <law at redhat dot com> 2013-02-08 20:06:24 UTC ---
Patch installed on trunk.