Re: An abridged "Writing C" for the gcc web pages
> On Apr 22, 2016, at 12:21 PM, Bernd Schmidt wrote: > > (Apologies if you get this twice, the mailing list didn't like the html > attachment in the first attempt). > > We frequently get malformatted patches, and it's been brought to my attention > that some people don't even make the effort to read the GNU coding standards > before trying to contribute code. TL;DR seems to be the excuse, and while I > find that attitude inappropriate, we could probably improve the situation by > spelling out the most basic rules in an abridged document on our webpages. > Below is a draft I came up with. Thoughts? Would you expect people to conform to the abridged version or the full standard? If the full standard, then publishing an abridged version is not a good idea, it will just cause confusion. Let the full standard be the rule, make people read it, and if they didn't bother that's their problem. paul
RE: PING^3: [PATCH]: New configure options that make the compiler use -fPIE and -pie as default option
From: gcc-patches-ow...@gcc.gnu.org [gcc-patches-ow...@gcc.gnu.org] on behalf of H.J. Lu [hjl.to...@gmail.com] Sent: Tuesday, May 19, 2015 11:27 AM To: Joseph Myers Cc: Magnus Granberg; GCC Patches Subject: Re: PING^3: [PATCH]: New configure options that make the compiler use -fPIE and -pie as default option On Tue, May 19, 2015 at 8:21 AM, Joseph Myers wrote: > ... > I think the whole thing should be posted as one patch, with both the > target-independent changes and the target-specific changes for all > targets. > That is what makes me concerned. I have some simple target-specified patches which weren't reviewed for years. What will happen if no one reviews some simple target-specified changes due to 1. Reviewers don't have access to those targets. 2. Target maintainers aren't review them. 3. There are no clear maintainers for those targets. As the result, my patch may go nowhere. --- But that hasn't stopped others from posting patches like that, or getting them approved. And we also have global maintainers who can approve things. It feels a bit like a hypothetical issue is being used as a reason to do part of the job. paul
Re: Remove LIBGCC2_HAS_?F_MODE target macros
On Sep 11, 2014, at 9:22 PM, Joseph S. Myers wrote:
> This patch removes the LIBGCC2_HAS_{SF,DF,XF,TF}_MODE target macros,
> replacing them by predefines with -fbuilding-libgcc, together with a
> target hook that can influence those predefines when needed.
>
> The new default is that a floating-point mode is supported in libgcc
> if (a) it passes the scalar_mode_supported_p hook (otherwise it's not
> plausible for it to be supported in libgcc) and (b) it's one of those
> four modes (since those are the modes for which libgcc hardcodes the
> possibility of support). The target hook can override the default
> choice (in either direction) for modes that pass
> scalar_mode_supported_p (although overriding in the direction of
> returning true when the default would return false only makes sense if
> all relevant functions are specially defined in libgcc for that
> particular target).
>
> The previous default settings depended on various settings such as
> LIBGCC2_LONG_DOUBLE_TYPE_SIZE, as well as targets defining the above
> target macros if the default wasn't correct.
>
> The default scalar_mode_supported_p only declares a floating-point
> mode to be supported if it matches one of float / double / long
> double. This means that in most cases where a mode is only supported
> conditionally in libgcc (TFmode only supported if it's the mode of
> long double, most commonly), the default gets things right. Overrides
> were needed in the following cases:
>
> * SFmode would always have been supported in libgcc (the condition was
> BITS_PER_UNIT == 8, true for all current targets), but pdp11
> defaults to 64-bit float, and in that case SFmode would fail
> scalar_mode_supported_p. I don't know if libgcc actually built for
> pdp11 (and the port may well no longer be being used), but this
> patch adds a scalar_mode_supported_p hook to it to ensure SFmode is
> treated as supported.
I thought it does build. I continue to work to keep that port alive.
The change looks fine.
The ideal solution, I think, would be to handle the choice of float length that
the pdp11 target has via the multilib machinery. Currently it does not do
that. If multilib were added for that at some point, would that require a
change of the code in that hook?
paul
Re: [PATCH 8/8] Add a common .md file and define standard constraints there
On Jun 12, 2014, at 3:24 PM, Segher Boessenkool wrote: > On Thu, Jun 05, 2014 at 10:43:25PM +0100, Richard Sandiford wrote: >> This final patch uses a common .md file to define all standard >> constraints except 'g'. > > I had a look at what targets still use "g". Note: there can be > errors in this, it's all based on \ > * frv and mcore use "g" in commented-out patterns; > * cr16, mcore, picochip, rl78, and sh use "g" where they mean "rm" > or "m"; > * m68k uses it (in a dbne pattern) where the C template splits > the "r", "m", "i" cases again; > * bfin, fr30, h8300, m68k, rs6000, and v850 use it as the second > operand (# bytes pushed) of the call patterns; that operand is > unused in all these cases, could just be ""; > * cris, m68k, pdp11, and vax actually use "g". > > So it won't be all that much work to completely get rid of "g". > Do we want that? Is it simply a matter of replacing “g” by “mri”? That’s what the doc suggests. Or is there more to the story than that? paul
Re: [Patch, RTL] Eliminate redundant vec_select moves.
On Dec 10, 2013, at 2:12 PM, H.J. Lu wrote: > On Tue, Dec 10, 2013 at 11:05 AM, Tejas Belagod wrote: >> ... >> So, if (subreg:DI (match_operand:V4SF 1 "register_operand" "x,x") 0) is a >> valid subreg, why not allow it in CANNOT_CHANGE_MODE_CLASS (like in Kirill's >> patch http://gcc.gnu.org/ml/gcc-patches/2013-12/msg00987.html) and resolve >> the actual register later in subreg_get_info ()? > > Let's wait for Kirill's results on GCC testsuite. I'm puzzled. What is the connection between testsuite results and a design decision about a code change? The question remains valid even if the testsuite didn't exist at all. paul
Re: [Patch, RTL] Eliminate redundant vec_select moves.
On Dec 10, 2013, at 9:50 AM, Kirill Yukhin wrote: > Hello, > On 09 Dec 14:08, H.J. Lu wrote: >> There are no regressions on Linux/x86-64 with -m32 and -m64. >> Can you check if it improves code quality on x886? > > That is exactly what I was talking about. However I wasn't sure > that we can change already defined (and used throughout ports) > target hook. > > ... > > Attached patch + updated test. You're missing the documentation change needed for this. paul
Re: Turn most genrecog warnings into errors
On Nov 22, 2013, at 3:43 AM, Richard Sandiford wrote: > genrecog does some useful sanity checks on the .md files. At the moment > it only reports most of the problems as warnings though, which means you > won't notice them unless you specifically look. > > I think the only message in validate_pattern that deserves to be a > warning is the one about missing modes, since the current code does > warn about valid cases. It would be good to tighten that up at some > point, but not today. > > Tested by building cc1 with and without the warning for each backend > (picking an arbitrary configuration triple in each case). The patch > below updates backends for which the fix was truly obvious: > removing constraints from things that don't allow constraints, > replacing one target-independent predicate with another that > doesn't accept immediates, or using match_operand rather than > match_test to test subpredicates. I've posted separate patches > for the backends that need changing in other ways. > > Also tested in the normal way on x86_64-linux-gnu. OK to install? > > Thanks, > Richard > > > gcc/ > * genrecog.c (validate_pattern): Treat all messages except missing > modes as errors. > * config/epiphany/epiphany.md: Remove constraints from > define_peephole2s. > * config/h8300/h8300.md: Remove constraints from define_splits. > * config/msp430/msp430.md: Likewise. > * config/mcore/mcore.md (movdi_i, movsf_i, movdf_k): Use > nonimmediate_operand rather than general_operand for operand 0. > * config/moxie/moxie.md (*movsi, *movqi, *movhi): Likewise. > * config/pdp11/predicates.md (float_operand, float_nonimm_operand): > Use match_operator rather than match_test to invoke general_operand. > * config/v850/v850.md (*movqi_internal, *movhi_internal) > (*movsi_internal_v850e, *movsi_internal, *movsf_internal): Likewise. I think you meant "use match_operand rather than ..." (in config/pdp11/predicates.md). paul
Re: PING^2: [patch] pr/54508: fix incomplete debug information for class
On Oct 23, 2012, at 2:02 PM, Jason Merrill wrote: > OK. > > Jason Thanks. Committed. paul
Re: PING^2: [patch] pr/54508: fix incomplete debug information for class
On Oct 5, 2012, at 6:05 PM, Cary Coutant wrote:
>> There certainly is a fair amount of code in dwarf2read.c in gdb to handle
>> DW_AT_declaration and do things differently for declarations.
>>
>> Should I rework this patch to use that mechanism instead? If so, how? If
>> the class is marked only by prune_unused_types_mark visiting it as a parent,
>> but hasn't been marked by ??? that visits all its children, then emit it
>> with a DW_AT_declaration marking?
>
> One question I'd consider is what do you want to see in the debugger
> if this truly is the only debug info you have for the class? (For
> example, in the test case you added, a DW_AT_declaration attribute
> won't help if there's no full definition of the class anywhere else.)
> Is it reasonable to just show a truncated class definition in that
> case, or do you want the full definition available. My tentative
> answer would be that we do the pruning here because we expect there to
> be a full definition somewhere else, and that the lack of a
> DW_AT_declaration attribute is the bug.
The answer appears to be:
1. Until the full symbols have been read (via gdb -r, or by reference to
another symbol in that compilation unit) such declarations are not visible.
Once the full symbols have been read, a class marked as DW_AT_declaration is
shown as "imcomplete type" which makes sense.
I think this is reasonable behavior. I confirmed that if you do have a
definition elsewhere, gdb does the correct thing. That's what you would
expect, given that the DW_AT_declaration flag was already being generated (for
imcomplete types).
>
> As you've discovered, however, it's not straightforward. You'll want
> to add the declaration attribute if you mark the DIE from below, but
> not from any reference where dokids is true. Alternatively, add the
> declaration attribute if any of its children are pruned. Perhaps that
> could be done in prune_unused_types_prune().
>
> If you're willing to rework the patch this way (assuming GDB does the
> right thing with it), I think that would be better. Thanks.
>
> -cary
Attached is the revised patch. It marks classes as "declaration" if they
weren't marked by one of the mark functions that visits children, and something
was actually pruned. That second check is needed, otherwise the class
"Executor" in testcase nested-3.C gets marked as a declaration.
The testcase has been reworked to check both aspects. Struct s gets defined
(because a variable of that type is defined), while class c and union u are
not, so they are marked as declaration while struct s is not marked. The
testcase verifies that.
Tested by build and check RUNTESTFLAGS="dwarf2.exp" on Linux and Darwin. Ok to
commit?
paul
ChangeLog:
2012-10-23 Paul Koning
* dwarf2out.c (prune_unused_types_prune): If pruning a class and
not all its children were marked, add DW_AT_declaration flag.
testcases/ChangeLog:
2012-10-23 Paul Koning
* g++.dg/debug/dwarf2/pr54508.C: New.
Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 192405)
+++ gcc/dwarf2out.c (working copy)
@@ -21218,6 +21218,7 @@
prune_unused_types_prune (dw_die_ref die)
{
dw_die_ref c;
+ int pruned = 0;
gcc_assert (die->die_mark);
prune_unused_types_update_strings (die);
@@ -21240,13 +21241,24 @@
prev->die_sib = c->die_sib;
die->die_child = prev;
}
- return;
+ pruned = 1;
+ goto finished;
}
if (c != prev->die_sib)
- prev->die_sib = c;
+ {
+ prev->die_sib = c;
+ pruned = 1;
+ }
prune_unused_types_prune (c);
} while (c != die->die_child);
+
+ finished:
+ /* If we pruned children, and this is a class, mark it as a
+ declaration to inform debuggers that this is not a complete
+ class definition. */
+ if (pruned && die->die_mark == 1 && class_scope_p (die))
+add_AT_flag (die, DW_AT_declaration, 1);
}
/* Remove dies representing declarations that we never use. */
Index: gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C
===
--- gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
+++ gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
@@ -0,0 +1,72 @@
+// PR debug/54508
+// { dg-do compile }
+// { dg-options "-g2 -dA -fno-merge-debug-strings" }
+
+// { dg-final { scan-assembler-not "\"cbase0\"\[ \t\]+\[#;/!|@\]+
DW_AT_name" } }
+// { dg-final { scan-assembler "\"c0\"\[ \t\]+\[#;/!|@\]+
DW_AT_name\[\r\n\]+\[\^\r\n\]+\[\r\n\]+\[\^\r\n\]+\[\r\n\]+\[\^#;/!|@\]+\[#;/!|@\]+
DW_AT_decl_line\[\r\n\]+\[\^#;/!|@\]+\[#;/!|@\]+ DW_AT_declaration" } }
+// { dg-final { scan-assembler-not "\"OPCODE0\"\[ \t\]+\[#;/!|@\]+
DW_AT_name" } }
+// { dg-final { scan-assembler-not "\"bi0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name"
} }
+// { dg-final { scan-assembler-not "\"si
Re: PING^2: [patch] pr/54508: fix incomplete debug information for class
On Oct 5, 2012, at 6:05 PM, Cary Coutant wrote: >> There certainly is a fair amount of code in dwarf2read.c in gdb to handle >> DW_AT_declaration and do things differently for declarations. >> >> Should I rework this patch to use that mechanism instead? If so, how? If >> the class is marked only by prune_unused_types_mark visiting it as a parent, >> but hasn't been marked by ??? that visits all its children, then emit it >> with a DW_AT_declaration marking? > > One question I'd consider is what do you want to see in the debugger > if this truly is the only debug info you have for the class? (For > example, in the test case you added, a DW_AT_declaration attribute > won't help if there's no full definition of the class anywhere else.) > Is it reasonable to just show a truncated class definition in that > case, or do you want the full definition available. My tentative > answer would be that we do the pruning here because we expect there to > be a full definition somewhere else, and that the lack of a > DW_AT_declaration attribute is the bug. > > As you've discovered, however, it's not straightforward. You'll want > to add the declaration attribute if you mark the DIE from below, but > not from any reference where dokids is true. Alternatively, add the > declaration attribute if any of its children are pruned. Perhaps that > could be done in prune_unused_types_prune(). > > If you're willing to rework the patch this way (assuming GDB does the > right thing with it), I think that would be better. Thanks. > > -cary I'll give it a try. I'll keep the existing patch in reserve in case we run out of time before Phase 1 end -- it would be good to have some sort of fix for this bug even if it's not the ideal one. paul
Re: PING^2: [patch] pr/54508: fix incomplete debug information for class
On Oct 5, 2012, at 2:43 PM, Cary Coutant wrote: >>> It seems to me that there are cases where we just want to emit the >>> class for the context info (like a namespace, which doesn't have to be >>> complete everywhere). Is there a way to tell the debugger that this >>> class declaration is incomplete and that it should look elsewhere for >>> a full definition? >> >> I think DW_AT_declaration would be appropriate. > > Yeah, that's what I was thinking. I was going to check with dje to see > if GDB would do the right thing in that case. > > -cary There certainly is a fair amount of code in dwarf2read.c in gdb to handle DW_AT_declaration and do things differently for declarations. Should I rework this patch to use that mechanism instead? If so, how? If the class is marked only by prune_unused_types_mark visiting it as a parent, but hasn't been marked by ??? that visits all its children, then emit it with a DW_AT_declaration marking? paul
Re: PING^2: [patch] pr/54508: fix incomplete debug information for class
On Oct 5, 2012, at 2:43 PM, Cary Coutant wrote: >>> It seems to me that there are cases where we just want to emit the >>> class for the context info (like a namespace, which doesn't have to be >>> complete everywhere). Is there a way to tell the debugger that this >>> class declaration is incomplete and that it should look elsewhere for >>> a full definition? >> >> I think DW_AT_declaration would be appropriate. > > Yeah, that's what I was thinking. I was going to check with dje to see > if GDB would do the right thing in that case. > > -cary Does that mean I should hold off committing this change? paul
Re: PING^2: [patch] pr/54508: fix incomplete debug information for class
On Oct 5, 2012, at 11:34 AM,
wrote:
>
> On Oct 5, 2012, at 4:16 AM, Jakub Jelinek wrote:
>
>> On Thu, Oct 04, 2012 at 05:26:11PM -0700, Cary Coutant wrote:
Index: gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C
===
--- gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (revision 192048)
+++ gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (working copy)
@@ -59,11 +59,11 @@
// { dg-final { scan-assembler "foo\[^\n\r\]*DW_AT_name" } }
// { dg-final { scan-assembler "staticfn1\[^\n\r\]*DW_AT_name" } }
// { dg-final { scan-assembler "staticfn2\[^\n\r\]*DW_AT_name" } }
-// { dg-final { scan-assembler-not "staticfn3\[^\n\r\]*DW_AT_name" } }
-// { dg-final { scan-assembler-not "staticfn4\[^\n\r\]*DW_AT_name" } }
+// { dg-final { scan-assembler "staticfn3\[^\n\r\]*DW_AT_name" } }
+// { dg-final { scan-assembler "staticfn4\[^\n\r\]*DW_AT_name" } }
// { dg-final { scan-assembler-not "staticfn5\[^\n\r\]*DW_AT_name" } }
// { dg-final { scan-assembler-not "staticfn6\[^\n\r\]*DW_AT_name" } }
-// { dg-final { scan-assembler-not "method1\[^\n\r\]*DW_AT_name" } }
+// { dg-final { scan-assembler "method1\[^\n\r\]*DW_AT_name" } }
// { dg-final { scan-assembler "arg1\[^\n\r\]*DW_AT_name" } }
// { dg-final { scan-assembler "arg2\[^\n\r\]*DW_AT_name" } }
// { dg-final { scan-assembler "arg3\[^\n\r\]*DW_AT_name" } }
>>>
>>> The fact that these two tests were specifically checking for the
>>> absence of staticfn3 and staticfn4 leads me to believe that the
>>> current behavior is deliberate. Jakub, that change was yours (it dates
>>> back to November 2008). Are you OK with Paul's change?
>>
>> Yes, thought it would be interesting to get some .debug_info size growth
>> numbers for a few projects (say libstdc++.so and some larger C++ codebase
>> (some KDE core library, or OO.o) without/with the patch, to see how much
>> does it bring with it (I'm not that much worried about the DW_TAG_subprogram
>> added itself, but about about types it will additionally bring in).
>> We have dwz and likely would get most of the growth back due to redundancy
>> removal though.
>>
>> Jakub
>
> I did a quick test on a large application of ours where this issue was
> causing trouble. Without the patch, it generates 115 MB of debug data; with
> the patch, that grows to 120 MB.
So given the comments, is this patch now ok to commit?
Thanks,
paul
Re: PING^2: [patch] pr/54508: fix incomplete debug information for class
On Oct 5, 2012, at 4:16 AM, Jakub Jelinek wrote:
> On Thu, Oct 04, 2012 at 05:26:11PM -0700, Cary Coutant wrote:
>>> Index: gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C
>>> ===
>>> --- gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (revision 192048)
>>> +++ gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (working copy)
>>> @@ -59,11 +59,11 @@
>>> // { dg-final { scan-assembler "foo\[^\n\r\]*DW_AT_name" } }
>>> // { dg-final { scan-assembler "staticfn1\[^\n\r\]*DW_AT_name" } }
>>> // { dg-final { scan-assembler "staticfn2\[^\n\r\]*DW_AT_name" } }
>>> -// { dg-final { scan-assembler-not "staticfn3\[^\n\r\]*DW_AT_name" } }
>>> -// { dg-final { scan-assembler-not "staticfn4\[^\n\r\]*DW_AT_name" } }
>>> +// { dg-final { scan-assembler "staticfn3\[^\n\r\]*DW_AT_name" } }
>>> +// { dg-final { scan-assembler "staticfn4\[^\n\r\]*DW_AT_name" } }
>>> // { dg-final { scan-assembler-not "staticfn5\[^\n\r\]*DW_AT_name" } }
>>> // { dg-final { scan-assembler-not "staticfn6\[^\n\r\]*DW_AT_name" } }
>>> -// { dg-final { scan-assembler-not "method1\[^\n\r\]*DW_AT_name" } }
>>> +// { dg-final { scan-assembler "method1\[^\n\r\]*DW_AT_name" } }
>>> // { dg-final { scan-assembler "arg1\[^\n\r\]*DW_AT_name" } }
>>> // { dg-final { scan-assembler "arg2\[^\n\r\]*DW_AT_name" } }
>>> // { dg-final { scan-assembler "arg3\[^\n\r\]*DW_AT_name" } }
>>
>> The fact that these two tests were specifically checking for the
>> absence of staticfn3 and staticfn4 leads me to believe that the
>> current behavior is deliberate. Jakub, that change was yours (it dates
>> back to November 2008). Are you OK with Paul's change?
>
> Yes, thought it would be interesting to get some .debug_info size growth
> numbers for a few projects (say libstdc++.so and some larger C++ codebase
> (some KDE core library, or OO.o) without/with the patch, to see how much
> does it bring with it (I'm not that much worried about the DW_TAG_subprogram
> added itself, but about about types it will additionally bring in).
> We have dwz and likely would get most of the growth back due to redundancy
> removal though.
>
> Jakub
I did a quick test on a large application of ours where this issue was causing
trouble. Without the patch, it generates 115 MB of debug data; with the patch,
that grows to 120 MB.
paul
Re: PING^2: [patch] pr/54508: fix incomplete debug information for class
On Oct 4, 2012, at 8:26 PM, Cary Coutant wrote:
>> Index: gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C
>> ===
>> --- gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (revision 192048)
>> +++ gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (working copy)
>> @@ -59,11 +59,11 @@
>> // { dg-final { scan-assembler "foo\[^\n\r\]*DW_AT_name" } }
>> // { dg-final { scan-assembler "staticfn1\[^\n\r\]*DW_AT_name" } }
>> // { dg-final { scan-assembler "staticfn2\[^\n\r\]*DW_AT_name" } }
>> -// { dg-final { scan-assembler-not "staticfn3\[^\n\r\]*DW_AT_name" } }
>> -// { dg-final { scan-assembler-not "staticfn4\[^\n\r\]*DW_AT_name" } }
>> +// { dg-final { scan-assembler "staticfn3\[^\n\r\]*DW_AT_name" } }
>> +// { dg-final { scan-assembler "staticfn4\[^\n\r\]*DW_AT_name" } }
>> // { dg-final { scan-assembler-not "staticfn5\[^\n\r\]*DW_AT_name" } }
>> // { dg-final { scan-assembler-not "staticfn6\[^\n\r\]*DW_AT_name" } }
>> -// { dg-final { scan-assembler-not "method1\[^\n\r\]*DW_AT_name" } }
>> +// { dg-final { scan-assembler "method1\[^\n\r\]*DW_AT_name" } }
>> // { dg-final { scan-assembler "arg1\[^\n\r\]*DW_AT_name" } }
>> // { dg-final { scan-assembler "arg2\[^\n\r\]*DW_AT_name" } }
>> // { dg-final { scan-assembler "arg3\[^\n\r\]*DW_AT_name" } }
>
> The fact that these two tests were specifically checking for the
> absence of staticfn3 and staticfn4 leads me to believe that the
> current behavior is deliberate. Jakub, that change was yours (it dates
> back to November 2008). Are you OK with Paul's change?
>
> It seems to me that there are cases where we just want to emit the
> class for the context info (like a namespace, which doesn't have to be
> complete everywhere). Is there a way to tell the debugger that this
> class declaration is incomplete and that it should look elsewhere for
> a full definition?
>
> -cary
The code itself (where I changed dwarf2out.c) is from 2003-02-28, by rth, what
appears to be the original implementation of the -feliminate-unused-debug-types
flag.
Looking at Jakub's change from 2008 and PR/27017 it fixes, it looks more like a
case of much more debug information that was missing and Jakub's change
corrected that. It looks like those two testcase files describe the resulting
behavior, but I don't read the discussion in PR/27017 as saying that having
"staticfn3" omitted was specifically desired.
paul
Re: PING^2: [patch] pr/54508: fix incomplete debug information for class
On Oct 4, 2012, at 8:26 PM, Cary Coutant wrote:
>> Index: gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C
>> ===
>> --- gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (revision 192048)
>> +++ gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (working copy)
>> @@ -59,11 +59,11 @@
>> // { dg-final { scan-assembler "foo\[^\n\r\]*DW_AT_name" } }
>> // { dg-final { scan-assembler "staticfn1\[^\n\r\]*DW_AT_name" } }
>> // { dg-final { scan-assembler "staticfn2\[^\n\r\]*DW_AT_name" } }
>> -// { dg-final { scan-assembler-not "staticfn3\[^\n\r\]*DW_AT_name" } }
>> -// { dg-final { scan-assembler-not "staticfn4\[^\n\r\]*DW_AT_name" } }
>> +// { dg-final { scan-assembler "staticfn3\[^\n\r\]*DW_AT_name" } }
>> +// { dg-final { scan-assembler "staticfn4\[^\n\r\]*DW_AT_name" } }
>> // { dg-final { scan-assembler-not "staticfn5\[^\n\r\]*DW_AT_name" } }
>> // { dg-final { scan-assembler-not "staticfn6\[^\n\r\]*DW_AT_name" } }
>> -// { dg-final { scan-assembler-not "method1\[^\n\r\]*DW_AT_name" } }
>> +// { dg-final { scan-assembler "method1\[^\n\r\]*DW_AT_name" } }
>> // { dg-final { scan-assembler "arg1\[^\n\r\]*DW_AT_name" } }
>> // { dg-final { scan-assembler "arg2\[^\n\r\]*DW_AT_name" } }
>> // { dg-final { scan-assembler "arg3\[^\n\r\]*DW_AT_name" } }
>
> The fact that these two tests were specifically checking for the
> absence of staticfn3 and staticfn4 leads me to believe that the
> current behavior is deliberate. Jakub, that change was yours (it dates
> back to November 2008). Are you OK with Paul's change?
>
> It seems to me that there are cases where we just want to emit the
> class for the context info (like a namespace, which doesn't have to be
> complete everywhere). Is there a way to tell the debugger that this
> class declaration is incomplete and that it should look elsewhere for
> a full definition?
>
> -cary
Certainly GDB does not currently do that. As it stands, it uses whatever
definition it finds first, so depending on which compilation unit's symbols are
read first, what you see varies unpredictably. If there is a way to express in
Dwarf-2 the fact that this is an incomplete definition, GDB could presumably be
changed to take advantage of that. I have no idea how hard that is.
paul
Re: PING^2: [patch] pr/54508: fix incomplete debug information for class
Updated patch: there were two existing testcases that needed to be adjusted
because of this fix.
Ran check RUNTESTFLAGS=dwarf2.exp, no regressions.
paul
ChangeLog:
2012-10-04 Paul Koning
* dwarf2out.c (prune_unused_types_mark): Mark all of parent's
children if parent is a class.
testsuite/ChangeLog:
2012-10-04 Paul Koning
* g++.dg/debug/dwarf2/pr54508.C: New.
* g++.dg/debug/dwarf2/localclass1.C: Expect staticfn1, staticfn2,
method1 in debug output.
* g++.dg/debug/dwarf2/localclass2.C: Likewise.
Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 192048)
+++ gcc/dwarf2out.c (working copy)
@@ -21035,9 +21035,11 @@
prune_unused_types_mark_generic_parms_dies (die);
/* We also have to mark its parents as used.
-(But we don't want to mark our parents' kids due to this.) */
+(But we don't want to mark our parent's kids due to this,
+unless it is a class.) */
if (die->die_parent)
- prune_unused_types_mark (die->die_parent, 0);
+ prune_unused_types_mark (die->die_parent,
+class_scope_p (die->die_parent));
/* Mark any referenced nodes. */
prune_unused_types_walk_attribs (die);
Index: gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C
===
--- gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
+++ gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
@@ -0,0 +1,67 @@
+// PR debug/54508
+// { dg-do compile }
+// { dg-options "-g2 -dA -fno-merge-debug-strings" }
+
+// { dg-final { scan-assembler "\"cbase0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name"
} }
+// { dg-final { scan-assembler "\"OPCODE0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name"
} }
+// { dg-final { scan-assembler "\"bi0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler "\"si0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler "\"f10\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler "\"f20\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler-not "\"nc0\"\[ \t\]+\# DW_AT_name" } }
+
+class cbase
+
+{
+public:
+ static int si;
+int bi;
+};
+
+class c : public cbase
+
+{
+public:
+ enum
+ {
+ OPCODE = 251
+ };
+ int i ;
+ static const char *testc (void) { return "foo"; }
+};
+
+struct s
+{
+int f1;
+static const char *tests (void) { return "test"; }
+};
+
+union u
+{
+int f2;
+double d;
+static const char *testu (void) { return "test union"; }
+};
+
+namespace n
+{
+const char *ntest (void) { return "test n"; }
+
+class nc
+{
+public:
+int i;
+static int sj;
+};
+}
+
+extern void send (int, int, const void *, int);
+
+void test (int src)
+{
+ int cookie = 1;
+ send(src, c::OPCODE, c::testc (), cookie);
+ send(src, c::OPCODE, s::tests (), cookie);
+ send(src, c::OPCODE, u::testu (), cookie);
+ send(src, c::OPCODE, n::ntest (), cookie);
+}
Index: gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C
===
--- gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (revision 192048)
+++ gcc/testsuite/g++.dg/debug/dwarf2/localclass1.C (working copy)
@@ -59,11 +59,11 @@
// { dg-final { scan-assembler "foo\[^\n\r\]*DW_AT_name" } }
// { dg-final { scan-assembler "staticfn1\[^\n\r\]*DW_AT_name" } }
// { dg-final { scan-assembler "staticfn2\[^\n\r\]*DW_AT_name" } }
-// { dg-final { scan-assembler-not "staticfn3\[^\n\r\]*DW_AT_name" } }
-// { dg-final { scan-assembler-not "staticfn4\[^\n\r\]*DW_AT_name" } }
+// { dg-final { scan-assembler "staticfn3\[^\n\r\]*DW_AT_name" } }
+// { dg-final { scan-assembler "staticfn4\[^\n\r\]*DW_AT_name" } }
// { dg-final { scan-assembler-not "staticfn5\[^\n\r\]*DW_AT_name" } }
// { dg-final { scan-assembler-not "staticfn6\[^\n\r\]*DW_AT_name" } }
-// { dg-final { scan-assembler-not "method1\[^\n\r\]*DW_AT_name" } }
+// { dg-final { scan-assembler "method1\[^\n\r\]*DW_AT_name" } }
// { dg-final { scan-assembler "arg1\[^\n\r\]*DW_AT_name" } }
// { dg-final { scan-assembler "arg2\[^\n\r\]*DW_AT_name" } }
// { dg-final { scan-assembler "arg3\[^\n\r\]*DW_AT_name" } }
Index: gcc/testsuite/g++.dg/debug/dwarf2/localclass2.C
===
--- gcc/testsuite/g++.dg/debug/dwarf2/localclass2.C (revision 192048)
+++ gcc/testsuite/g++.dg/debug/dwarf2/localclass2.C (working copy)
@@ -59,11 +59,11 @@
// { dg-final { scan-assembler "foo\[^\n\r\]*DW_AT_name" } }
// { dg-final { scan-assembler "staticfn1\[^\n\r\]*DW_AT_name" } }
// { dg-final { scan-assembler "staticfn2\[^\n\r\]*DW_AT_name" } }
-// { dg-final { scan-assembler-not "staticfn3\[^\n\r\]*DW_AT_name" } }
-// { dg-final { scan-assembler-not "staticfn4\[^\n\r\]*DW_AT_name" } }
+// { dg-final { scan-assembler "staticfn3\[^\
Re: PING^2: [patch] pr/54508: fix incomplete debug information for class
On Oct 4, 2012, at 12:09 PM, wrote: > Ping ^ 2... > > -- > > If the only reference in a source file is to a static method of a class, then > GCC would output debug information for the class name but not any of its > members or base classes. The attached patch fixes this by having > "prune_unused_types_mark" mark all of the parent's children if the parent DIE > type is for a class. > > The associated new testcase verifies this, and also verifies this that > references to a function in a namespace do *not* cause other parts of that > namespace to be emitted as debug information, but that references to a method > in a class (or struct or union) do emit the other information for that class. > > Checked by "make check" on dwarf2.exp. My apologies -- I missed something when I did that check. Some existing testcases need to be adjusted to match. Updated patch coming shortly. paul
Re: PING^2: [patch] pr/54508: fix incomplete debug information for class
On Oct 4, 2012, at 2:58 PM, Jakub Jelinek wrote:
> On Thu, Oct 04, 2012 at 06:32:20PM +, paul_kon...@dell.com wrote:
>> --- testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
>> +++ testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
>> @@ -0,0 +1,67 @@
>> +// PR debug/54508
>> +// { dg-do compile }
>> +// { dg-options "-g2 -dA" }
>
> Better than this just add -fno-merge-debug-strings to dg-options
>
>> +
>> +// { dg-final { scan-assembler "\"cbase0\"\[ \t\]+\[#;/!|@\]+
>> DW_AT_name\|DW_AT_name: \"cbase\"" } }
>> +// { dg-final { scan-assembler "\"OPCODE0\"\[ \t\]+\[#;/!|@\]+
>> DW_AT_name\|DW_AT_name: \"OPCODE\"" } }
>> +// { dg-final { scan-assembler "\"bi0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name"
>> } }
>> +// { dg-final { scan-assembler "\"si0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name"
>> } }
>> +// { dg-final { scan-assembler "\"f10\"\[ \t\]+\[#;/!|@\]+ DW_AT_name"
>> } }
>> +// { dg-final { scan-assembler "\"f20\"\[ \t\]+\[#;/!|@\]+ DW_AT_name"
>> } }
>> +// { dg-final { scan-assembler-not "\"nc0\"\[ \t\]+\#
>> DW_AT_name\|DW_AT_name: \"nc\"" } }
>
> and use just one of the alternatives in the regexps.
>
> Jakub
Thanks... learn something new every day.
Here is the updated patch.
paul
ChangeLog:
2012-10-04 Paul Koning
* dwarf2out.c (prune_unused_types_mark): Mark all of parent's
children if parent is a class.
testsuite/ChangeLog:
2012-10-04 Paul Koning
* g++.dg/debug/dwarf2/pr54508.C: New.
Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 192048)
+++ gcc/dwarf2out.c (working copy)
@@ -21035,9 +21035,11 @@
prune_unused_types_mark_generic_parms_dies (die);
/* We also have to mark its parents as used.
-(But we don't want to mark our parents' kids due to this.) */
+(But we don't want to mark our parent's kids due to this,
+unless it is a class.) */
if (die->die_parent)
- prune_unused_types_mark (die->die_parent, 0);
+ prune_unused_types_mark (die->die_parent,
+class_scope_p (die->die_parent));
/* Mark any referenced nodes. */
prune_unused_types_walk_attribs (die);
Index: gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C
===
--- gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
+++ gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
@@ -0,0 +1,67 @@
+// PR debug/54508
+// { dg-do compile }
+// { dg-options "-g2 -dA -fno-merge-debug-strings" }
+
+// { dg-final { scan-assembler "\"cbase0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name"
} }
+// { dg-final { scan-assembler "\"OPCODE0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name"
} }
+// { dg-final { scan-assembler "\"bi0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler "\"si0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler "\"f10\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler "\"f20\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler-not "\"nc0\"\[ \t\]+\# DW_AT_name" } }
+
+class cbase
+
+{
+public:
+ static int si;
+int bi;
+};
+
+class c : public cbase
+
+{
+public:
+ enum
+ {
+ OPCODE = 251
+ };
+ int i ;
+ static const char *testc (void) { return "foo"; }
+};
+
+struct s
+{
+int f1;
+static const char *tests (void) { return "test"; }
+};
+
+union u
+{
+int f2;
+double d;
+static const char *testu (void) { return "test union"; }
+};
+
+namespace n
+{
+const char *ntest (void) { return "test n"; }
+
+class nc
+{
+public:
+int i;
+static int sj;
+};
+}
+
+extern void send (int, int, const void *, int);
+
+void test (int src)
+{
+ int cookie = 1;
+ send(src, c::OPCODE, c::testc (), cookie);
+ send(src, c::OPCODE, s::tests (), cookie);
+ send(src, c::OPCODE, u::testu (), cookie);
+ send(src, c::OPCODE, n::ntest (), cookie);
+}
Re: PING^2: [patch] pr/54508: fix incomplete debug information for class
On Oct 4, 2012, at 1:38 PM, Cary Coutant wrote:
>> /* We also have to mark its parents as used.
>> -(But we don't want to mark our parents' kids due to this.) */
>> +(But we don't want to mark our parent's kids due to this,
>> +unless it is a class.) */
>> if (die->die_parent)
>> - prune_unused_types_mark (die->die_parent, 0);
>> + prune_unused_types_mark (die->die_parent,
>> +(die->die_parent->die_tag ==
>> DW_TAG_class_type ||
>> + die->die_parent->die_tag ==
>> DW_TAG_structure_type ||
>> + die->die_parent->die_tag ==
>> DW_TAG_union_type));
>
> I'd suggest replacing these conditions with a call to class_scope_p().
> That will also cover DW_TAG_interface_type, which might be irrelevant
> for this particular case, but is probably good to cover in the general
> case.
>
> -cary
Thanks, that makes it very simple. Here is the updated patch.
Ok to commit with this change?
paul
ChangeLog:
2012-10-04 Paul Koning
* dwarf2out.c (prune_unused_types_mark): Mark all of parent's
children if parent is a class.
testsuite/ChangeLog:
2012-10-04 Paul Koning
* g++.dg/debug/dwarf2/pr54508.C: New.
Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 192048)
+++ gcc/dwarf2out.c (working copy)
@@ -21035,9 +21035,11 @@
prune_unused_types_mark_generic_parms_dies (die);
/* We also have to mark its parents as used.
-(But we don't want to mark our parents' kids due to this.) */
+(But we don't want to mark our parent's kids due to this,
+unless it is a class.) */
if (die->die_parent)
- prune_unused_types_mark (die->die_parent, 0);
+ prune_unused_types_mark (die->die_parent,
+class_scope_p (die->die_parent));
/* Mark any referenced nodes. */
prune_unused_types_walk_attribs (die);
Index: testsuite/g++.dg/debug/dwarf2/pr54508.C
===
--- testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
+++ testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
@@ -0,0 +1,67 @@
+// PR debug/54508
+// { dg-do compile }
+// { dg-options "-g2 -dA" }
+
+// { dg-final { scan-assembler "\"cbase0\"\[ \t\]+\[#;/!|@\]+
DW_AT_name\|DW_AT_name: \"cbase\"" } }
+// { dg-final { scan-assembler "\"OPCODE0\"\[ \t\]+\[#;/!|@\]+
DW_AT_name\|DW_AT_name: \"OPCODE\"" } }
+// { dg-final { scan-assembler "\"bi0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler "\"si0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler "\"f10\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler "\"f20\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler-not "\"nc0\"\[ \t\]+\#
DW_AT_name\|DW_AT_name: \"nc\"" } }
+
+class cbase
+
+{
+public:
+ static int si;
+int bi;
+};
+
+class c : public cbase
+
+{
+public:
+ enum
+ {
+ OPCODE = 251
+ };
+ int i ;
+ static const char *testc (void) { return "foo"; }
+};
+
+struct s
+{
+int f1;
+static const char *tests (void) { return "test"; }
+};
+
+union u
+{
+int f2;
+double d;
+static const char *testu (void) { return "test union"; }
+};
+
+namespace n
+{
+const char *ntest (void) { return "test n"; }
+
+class nc
+{
+public:
+int i;
+static int sj;
+};
+}
+
+extern void send (int, int, const void *, int);
+
+void test (int src)
+{
+ int cookie = 1;
+ send(src, c::OPCODE, c::testc (), cookie);
+ send(src, c::OPCODE, s::tests (), cookie);
+ send(src, c::OPCODE, u::testu (), cookie);
+ send(src, c::OPCODE, n::ntest (), cookie);
+}
PING^2: [patch] pr/54508: fix incomplete debug information for class
Ping ^ 2...
--
If the only reference in a source file is to a static method of a class, then
GCC would output debug information for the class name but not any of its
members or base classes. The attached patch fixes this by having
"prune_unused_types_mark" mark all of the parent's children if the parent DIE
type is for a class.
The associated new testcase verifies this, and also verifies this that
references to a function in a namespace do *not* cause other parts of that
namespace to be emitted as debug information, but that references to a method
in a class (or struct or union) do emit the other information for that class.
Checked by "make check" on dwarf2.exp.
Ok to commit? This would close PR/54508.
paul
ChangeLog:
2012-09-17 Paul Koning
* dwarf2out.c (prune_unused_types_mark): Mark all of parent's
children if parent is a class.
testsuite/ChangeLog:
2012-09-17 Paul Koning
* g++.dg/debug/dwarf2/pr54508.C: New.
Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 191408)
+++ gcc/dwarf2out.c (working copy)
@@ -21033,9 +21033,13 @@
prune_unused_types_mark_generic_parms_dies (die);
/* We also have to mark its parents as used.
-(But we don't want to mark our parents' kids due to this.) */
+(But we don't want to mark our parent's kids due to this,
+unless it is a class.) */
if (die->die_parent)
- prune_unused_types_mark (die->die_parent, 0);
+ prune_unused_types_mark (die->die_parent,
+(die->die_parent->die_tag == DW_TAG_class_type
||
+ die->die_parent->die_tag ==
DW_TAG_structure_type ||
+ die->die_parent->die_tag ==
DW_TAG_union_type));
/* Mark any referenced nodes. */
prune_unused_types_walk_attribs (die);
Index: testsuite/g++.dg/debug/dwarf2/pr54508.C
===
--- testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
+++ testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
@@ -0,0 +1,67 @@
+// PR debug/54508
+// { dg-do compile }
+// { dg-options "-g2 -dA" }
+
+// { dg-final { scan-assembler "\"cbase0\"\[ \t\]+\[#;/!|@\]+
DW_AT_name\|DW_AT_name: \"cbase\"" } }
+// { dg-final { scan-assembler "\"OPCODE0\"\[ \t\]+\[#;/!|@\]+
DW_AT_name\|DW_AT_name: \"OPCODE\"" } }
+// { dg-final { scan-assembler "\"bi0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler "\"si0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler "\"f10\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler "\"f20\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
+// { dg-final { scan-assembler-not "\"nc0\"\[ \t\]+\#
DW_AT_name\|DW_AT_name: \"nc\"" } }
+
+class cbase
+
+{
+public:
+ static int si;
+int bi;
+};
+
+class c : public cbase
+
+{
+public:
+ enum
+ {
+ OPCODE = 251
+ };
+ int i ;
+ static const char *testc (void) { return "foo"; }
+};
+
+struct s
+{
+int f1;
+static const char *tests (void) { return "test"; }
+};
+
+union u
+{
+int f2;
+double d;
+static const char *testu (void) { return "test union"; }
+};
+
+namespace n
+{
+const char *ntest (void) { return "test n"; }
+
+class nc
+{
+public:
+int i;
+static int sj;
+};
+}
+
+extern void send (int, int, const void *, int);
+
+void test (int src)
+{
+ int cookie = 1;
+ send(src, c::OPCODE, c::testc (), cookie);
+ send(src, c::OPCODE, s::tests (), cookie);
+ send(src, c::OPCODE, u::testu (), cookie);
+ send(src, c::OPCODE, n::ntest (), cookie);
+}
Re: RFC: LRA for x86/x86-64 [4/9]
On Oct 1, 2012, at 2:51 PM, Richard Sandiford wrote: > ... > E.g. for MIPS, SImode loads and stores have a displacement range of > [-32768, 32764], but DImode loads and stores only accept [-32768, 32760]. > So the maximal displacement depends on mode, even though the instruction set > is pretty regular. It may be that the case doesn't arise in code GCC generates, but I don't think that's true. The offset field is always a 2's complement 16 bit integer, hence in the range -32768..32767. The alignment required in loading multibyte data with aligned load/store instructions applies to the final address, not the offset. For example, if R1 contains 1, then LD r2,32767(r1) will work. paul
Re: [PATCH v2, rtl-optimization]: Fix PR54457, [x32] Fail to combine 64bit index + constant
On Sep 27, 2012, at 2:04 PM, Uros Bizjak wrote: > > > > I agree (subreg:M (op:N A C) 0) to (op:M (subreg:N (A 0)) C) is > a good transformation, but why do we need to handle as special > the case where the subreg is itself the operand of a plus or minus? > I think it should happen regardless of where the subreg occurs. Don't we need to restrict this to the low part though? >>> >>> ... > > After some off-line discussion with Richard, attached is v2 of the patch. > > 2012-09-27 Uros Bizjak > >PR rtl-optimization/54457 >* simplify-rtx.c (simplify_subreg): > Simplify (subreg:SI (op:DI ((x:DI) (y:DI)), 0) > to (op:SI (subreg:SI (x:DI) 0) (subreg:SI (x:DI) 0)). > ... Is it just specific to DI -> SI, or is it for any large mode -> smaller mode, like SI -> HI? paul
Ping: [patch] pr/54508: fix incomplete debug information for class
Ping...
paul
Begin forwarded message:
> From:
> Date: September 20, 2012 4:55:05 PM EDT
> To:
> Subject: Re: [patch] pr/54508: fix incomplete debug information for class
>
> Attached below is an update to the testcase file, to fix the scan-assembler
> regexp comment character matching. This uses the same regexp element that
> H-P Nilsson used in the fix to nested-3.C two days ago.
>
> By the way, I ran check-g++ for dwarf2.exp on this change, no regressions.
>
> Ok to commit?
>
> paul
>
> On Sep 17, 2012, at 5:24 PM,
> wrote:
>
>> If the only reference in a source file is to a static method of a class,
>> then GCC would output debug information for the class name but not any of
>> its members or base classes. The attached patch fixes this by having
>> "prune_unused_types_mark" mark all of the parent's children if the parent
>> DIE type is for a class.
>>
>> The associated new testcase verifies this, and also verifies this that
>> references to a function in a namespace do *not* cause other parts of that
>> namespace to be emitted as debug information, but that references to a
>> method in a class (or struct or union) do emit the other information for
>> that class.
>>
>> Ok to commit? This would close PR/54508.
>>
>> paul
>>
>> ChangeLog:
>>
>> 2012-09-17 Paul Koning
>>
>> * dwarf2out.c (prune_unused_types_mark): Mark all of parent's
>> children if parent is a class.
>>
>> testsuite/ChangeLog:
>>
>> 2012-09-17 Paul Koning
>>
>> * g++.dg/debug/dwarf2/pr54508.C: New.
>>
>> Index: gcc/dwarf2out.c
>> ===
>> --- gcc/dwarf2out.c (revision 191408)
>> +++ gcc/dwarf2out.c (working copy)
>> @@ -21033,9 +21033,13 @@
>> prune_unused_types_mark_generic_parms_dies (die);
>>
>> /* We also have to mark its parents as used.
>> - (But we don't want to mark our parents' kids due to this.) */
>> + (But we don't want to mark our parent's kids due to this,
>> + unless it is a class.) */
>> if (die->die_parent)
>> -prune_unused_types_mark (die->die_parent, 0);
>> +prune_unused_types_mark (die->die_parent,
>> + (die->die_parent->die_tag == DW_TAG_class_type
>> ||
>> + die->die_parent->die_tag ==
>> DW_TAG_structure_type ||
>> + die->die_parent->die_tag ==
>> DW_TAG_union_type));
>>
>> /* Mark any referenced nodes. */
>> prune_unused_types_walk_attribs (die);
>> Index: gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C
>> ===
>> --- gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
>> +++ gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
>> @@ -0,0 +1,67 @@
>> +// PR debug/54508
>> +// { dg-do compile }
>> +// { dg-options "-g2 -dA" }
>> +
>> +// { dg-final { scan-assembler "\"cbase0\"\[ \t\]+\#
>> DW_AT_name\|DW_AT_name: \"cbase\"" } }
>> +// { dg-final { scan-assembler "\"OPCODE0\"\[ \t\]+\#
>> DW_AT_name\|DW_AT_name: \"OPCODE\"" } }
>> +// { dg-final { scan-assembler "\"bi0\"\[ \t\]+\# DW_AT_name" } }
>> +// { dg-final { scan-assembler "\"si0\"\[ \t\]+\# DW_AT_name" } }
>> +// { dg-final { scan-assembler "\"f10\"\[ \t\]+\# DW_AT_name" } }
>> +// { dg-final { scan-assembler "\"f20\"\[ \t\]+\# DW_AT_name" } }
>> +// { dg-final { scan-assembler-not "\"nc0\"\[ \t\]+\#
>> DW_AT_name\|DW_AT_name: \"nc\"" } }
>> +...
>
> // PR debug/54508
> // { dg-do compile }
> // { dg-options "-g2 -dA" }
>
> // { dg-final { scan-assembler "\"cbase0\"\[ \t\]+\[#;/!|@\]+
> DW_AT_name\|DW_AT_name: \"cbase\"" } }
> // { dg-final { scan-assembler "\"OPCODE0\"\[ \t\]+\[#;/!|@\]+
> DW_AT_name\|DW_AT_name: \"OPCODE\"" } }
> // { dg-final { scan-assembler "\"bi0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
> // { dg-final { scan-assembler "\"si0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
> // { dg-final { scan-assembler "\"f10\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
> // { dg-final { scan-assembler "\"f20\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
> // { dg-final { scan-assembler-not "\"nc0\"\[ \t\]+\#
> DW_AT_name\|DW_AT_name: \"nc\"" } }
>
> class cbase
>
> {
> public:
> static int si;
>int bi;
> };
>
> class c : public cbase
>
> {
> public:
> enum
> {
> OPCODE = 251
> };
> int i ;
> static const char *testc (void) { return "foo"; }
> };
>
> struct s
> {
>int f1;
>static const char *tests (void) { return "test"; }
> };
>
> union u
> {
>int f2;
>double d;
>static const char *testu (void) { return "test union"; }
> };
>
> namespace n
> {
>const char *ntest (void) { return "test n"; }
>
>class nc
>{
>public:
>int i;
>static int sj;
>};
> }
>
> extern void send (int, int, const void *, int);
>
> void test (int src)
> {
> int cookie = 1;
> send(src, c::OPCODE, c::testc (), cookie);
> send
Re: [patch] pr/54508: fix incomplete debug information for class
Attached below is an update to the testcase file, to fix the scan-assembler
regexp comment character matching. This uses the same regexp element that H-P
Nilsson used in the fix to nested-3.C two days ago.
By the way, I ran check-g++ for dwarf2.exp on this change, no regressions.
Ok to commit?
paul
On Sep 17, 2012, at 5:24 PM,
wrote:
> If the only reference in a source file is to a static method of a class, then
> GCC would output debug information for the class name but not any of its
> members or base classes. The attached patch fixes this by having
> "prune_unused_types_mark" mark all of the parent's children if the parent DIE
> type is for a class.
>
> The associated new testcase verifies this, and also verifies this that
> references to a function in a namespace do *not* cause other parts of that
> namespace to be emitted as debug information, but that references to a method
> in a class (or struct or union) do emit the other information for that class.
>
> Ok to commit? This would close PR/54508.
>
> paul
>
> ChangeLog:
>
> 2012-09-17 Paul Koning
>
> * dwarf2out.c (prune_unused_types_mark): Mark all of parent's
> children if parent is a class.
>
> testsuite/ChangeLog:
>
> 2012-09-17 Paul Koning
>
> * g++.dg/debug/dwarf2/pr54508.C: New.
>
> Index: gcc/dwarf2out.c
> ===
> --- gcc/dwarf2out.c (revision 191408)
> +++ gcc/dwarf2out.c (working copy)
> @@ -21033,9 +21033,13 @@
> prune_unused_types_mark_generic_parms_dies (die);
>
> /* We also have to mark its parents as used.
> - (But we don't want to mark our parents' kids due to this.) */
> + (But we don't want to mark our parent's kids due to this,
> + unless it is a class.) */
> if (die->die_parent)
> - prune_unused_types_mark (die->die_parent, 0);
> + prune_unused_types_mark (die->die_parent,
> + (die->die_parent->die_tag == DW_TAG_class_type
> ||
> + die->die_parent->die_tag ==
> DW_TAG_structure_type ||
> + die->die_parent->die_tag ==
> DW_TAG_union_type));
>
> /* Mark any referenced nodes. */
> prune_unused_types_walk_attribs (die);
> Index: gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C
> ===
> --- gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
> +++ gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
> @@ -0,0 +1,67 @@
> +// PR debug/54508
> +// { dg-do compile }
> +// { dg-options "-g2 -dA" }
> +
> +// { dg-final { scan-assembler "\"cbase0\"\[ \t\]+\#
> DW_AT_name\|DW_AT_name: \"cbase\"" } }
> +// { dg-final { scan-assembler "\"OPCODE0\"\[ \t\]+\#
> DW_AT_name\|DW_AT_name: \"OPCODE\"" } }
> +// { dg-final { scan-assembler "\"bi0\"\[ \t\]+\# DW_AT_name" } }
> +// { dg-final { scan-assembler "\"si0\"\[ \t\]+\# DW_AT_name" } }
> +// { dg-final { scan-assembler "\"f10\"\[ \t\]+\# DW_AT_name" } }
> +// { dg-final { scan-assembler "\"f20\"\[ \t\]+\# DW_AT_name" } }
> +// { dg-final { scan-assembler-not "\"nc0\"\[ \t\]+\#
> DW_AT_name\|DW_AT_name: \"nc\"" } }
> +...
// PR debug/54508
// { dg-do compile }
// { dg-options "-g2 -dA" }
// { dg-final { scan-assembler "\"cbase0\"\[ \t\]+\[#;/!|@\]+
DW_AT_name\|DW_AT_name: \"cbase\"" } }
// { dg-final { scan-assembler "\"OPCODE0\"\[ \t\]+\[#;/!|@\]+
DW_AT_name\|DW_AT_name: \"OPCODE\"" } }
// { dg-final { scan-assembler "\"bi0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
// { dg-final { scan-assembler "\"si0\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
// { dg-final { scan-assembler "\"f10\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
// { dg-final { scan-assembler "\"f20\"\[ \t\]+\[#;/!|@\]+ DW_AT_name" } }
// { dg-final { scan-assembler-not "\"nc0\"\[ \t\]+\#
DW_AT_name\|DW_AT_name: \"nc\"" } }
class cbase
{
public:
static int si;
int bi;
};
class c : public cbase
{
public:
enum
{
OPCODE = 251
};
int i ;
static const char *testc (void) { return "foo"; }
};
struct s
{
int f1;
static const char *tests (void) { return "test"; }
};
union u
{
int f2;
double d;
static const char *testu (void) { return "test union"; }
};
namespace n
{
const char *ntest (void) { return "test n"; }
class nc
{
public:
int i;
static int sj;
};
}
extern void send (int, int, const void *, int);
void test (int src)
{
int cookie = 1;
send(src, c::OPCODE, c::testc (), cookie);
send(src, c::OPCODE, s::tests (), cookie);
send(src, c::OPCODE, u::testu (), cookie);
send(src, c::OPCODE, n::ntest (), cookie);
}
[patch] pr/54508: fix incomplete debug information for class
If the only reference in a source file is to a static method of a class, then
GCC would output debug information for the class name but not any of its
members or base classes. The attached patch fixes this by having
"prune_unused_types_mark" mark all of the parent's children if the parent DIE
type is for a class.
The associated new testcase verifies this, and also verifies this that
references to a function in a namespace do *not* cause other parts of that
namespace to be emitted as debug information, but that references to a method
in a class (or struct or union) do emit the other information for that class.
Ok to commit? This would close PR/54508.
paul
ChangeLog:
2012-09-17 Paul Koning
* dwarf2out.c (prune_unused_types_mark): Mark all of parent's
children if parent is a class.
testsuite/ChangeLog:
2012-09-17 Paul Koning
* g++.dg/debug/dwarf2/pr54508.C: New.
Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 191408)
+++ gcc/dwarf2out.c (working copy)
@@ -21033,9 +21033,13 @@
prune_unused_types_mark_generic_parms_dies (die);
/* We also have to mark its parents as used.
-(But we don't want to mark our parents' kids due to this.) */
+(But we don't want to mark our parent's kids due to this,
+unless it is a class.) */
if (die->die_parent)
- prune_unused_types_mark (die->die_parent, 0);
+ prune_unused_types_mark (die->die_parent,
+(die->die_parent->die_tag == DW_TAG_class_type
||
+ die->die_parent->die_tag ==
DW_TAG_structure_type ||
+ die->die_parent->die_tag ==
DW_TAG_union_type));
/* Mark any referenced nodes. */
prune_unused_types_walk_attribs (die);
Index: gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C
===
--- gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
+++ gcc/testsuite/g++.dg/debug/dwarf2/pr54508.C (revision 0)
@@ -0,0 +1,67 @@
+// PR debug/54508
+// { dg-do compile }
+// { dg-options "-g2 -dA" }
+
+// { dg-final { scan-assembler "\"cbase0\"\[ \t\]+\#
DW_AT_name\|DW_AT_name: \"cbase\"" } }
+// { dg-final { scan-assembler "\"OPCODE0\"\[ \t\]+\#
DW_AT_name\|DW_AT_name: \"OPCODE\"" } }
+// { dg-final { scan-assembler "\"bi0\"\[ \t\]+\# DW_AT_name" } }
+// { dg-final { scan-assembler "\"si0\"\[ \t\]+\# DW_AT_name" } }
+// { dg-final { scan-assembler "\"f10\"\[ \t\]+\# DW_AT_name" } }
+// { dg-final { scan-assembler "\"f20\"\[ \t\]+\# DW_AT_name" } }
+// { dg-final { scan-assembler-not "\"nc0\"\[ \t\]+\#
DW_AT_name\|DW_AT_name: \"nc\"" } }
+
+class cbase
+
+{
+public:
+ static int si;
+int bi;
+};
+
+class c : public cbase
+
+{
+public:
+ enum
+ {
+ OPCODE = 251
+ };
+ int i ;
+ static const char *testc (void) { return "foo"; }
+};
+
+struct s
+{
+int f1;
+static const char *tests (void) { return "test"; }
+};
+
+union u
+{
+int f2;
+double d;
+static const char *testu (void) { return "test union"; }
+};
+
+namespace n
+{
+const char *ntest (void) { return "test n"; }
+
+class nc
+{
+public:
+int i;
+static int sj;
+};
+}
+
+extern void send (int, int, const void *, int);
+
+void test (int src)
+{
+ int cookie = 1;
+ send(src, c::OPCODE, c::testc (), cookie);
+ send(src, c::OPCODE, s::tests (), cookie);
+ send(src, c::OPCODE, u::testu (), cookie);
+ send(src, c::OPCODE, n::ntest (), cookie);
+}
Re: [DOC] Update -feliminate-unused-debug-types description
On Sep 17, 2012, at 1:20 PM, Ian Lance Taylor wrote: > On Mon, Sep 17, 2012 at 8:59 AM, wrote: >> >> 2012-09-17 Paul Koning >> >>* doc/invoke.text (-feliminate-unused-debug-types): Update to >>reflect that this is enabled by default. > > This is OK. > > Thanks. > > Ian Thanks. Committed. paul
[DOC] Update -feliminate-unused-debug-types description
Currently the description of -feliminate-unused-debug-types says that it is off
by default. In fact, it is on by default. The attached patch corrects the
documentation to reflect that.
Ok to commit?
paul
2012-09-17 Paul Koning
* doc/invoke.text (-feliminate-unused-debug-types): Update to
reflect that this is enabled by default.
Index: invoke.texi
===
--- invoke.texi (revision 191393)
+++ invoke.texi (working copy)
@@ -324,7 +324,7 @@
-fdump-tree-storeccp@r{[}-@var{n}@r{]} @gol
-fdump-final-insns=@var{file} @gol
-fcompare-debug@r{[}=@var{opts}@r{]} -fcompare-debug-second @gol
--feliminate-dwarf2-dups -feliminate-unused-debug-types @gol
+-feliminate-dwarf2-dups -fno-eliminate-unused-debug-types @gol
-feliminate-unused-debug-symbols -femit-class-debug-always @gol
-fenable-@var{kind}-@var{pass} @gol
-fenable-@var{kind}-@var{pass}=@var{range-list} @gol
@@ -6192,17 +6192,18 @@
Print the compiler's built-in specs---and don't do anything else. (This
is used when GCC itself is being built.) @xref{Spec Files}.
-@item -feliminate-unused-debug-types
+@item -fno-eliminate-unused-debug-types
@opindex feliminate-unused-debug-types
-Normally, when producing DWARF 2 output, GCC emits debugging
+@opindex fno-eliminate-unused-debug-types
+Normally, when producing DWARF 2 output, GCC avoids producing debug symbol
+output for types that are nowhere used in the source file being compiled.
+Sometimes it is useful to have GCC emit debugging
information for all types declared in a compilation
unit, regardless of whether or not they are actually used
-in that compilation unit. Sometimes this is useful, such as
+in that compilation unit, for example
if, in the debugger, you want to cast a value to a type that is
not actually used in your program (but is declared). More often,
however, this results in a significant amount of wasted space.
-With this option, GCC avoids producing debug symbol output
-for types that are nowhere used in the source file being compiled.
@end table
@node Optimize Options
Re: [patch] For alpha-vms, unset flag_jump_tables if flag_pic is nonzero
On May 16, 2012, at 1:27 PM, wrote: > It turns out that there is a second target that doesn't define > ASM_OUTPUT_ADDR_DIFF_ELT: pdp11. I'll fix that. > > The documentation is out of date as a consequence of this patch, because it > still says that ASM_OUTPUT_ADDR_DIFF_ELT is optional which is no longer the > case (or not unless you take some other actions as you did in the VAX port). > > paul > Now I'm really confused. I defined the missing macro, and now I no longer get an ICE -- but I don't see any output from that macro either. No jump tables with -fpic, instead I get what looks like the no table jump available case (decision tree). Why would that be? paul
Re: [patch] For alpha-vms, unset flag_jump_tables if flag_pic is nonzero
It turns out that there is a second target that doesn't define ASM_OUTPUT_ADDR_DIFF_ELT: pdp11. I'll fix that. The documentation is out of date as a consequence of this patch, because it still says that ASM_OUTPUT_ADDR_DIFF_ELT is optional which is no longer the case (or not unless you take some other actions as you did in the VAX port). paul
RE: [patch] clean up pdp11.md a bit
By the way, the "compile" subset of the testsuite works for pdp11; it has some errors which still need cleanup but a large fraction works. paul -Original Message- From: Steven Bosscher [mailto:stevenb@gmail.com] Sent: Tuesday, February 14, 2012 5:09 PM To: Koning, Paul; GCC Patches Subject: [patch] clean up pdp11.md a bit Hello, Just a few cleanups for things I noticed last weekend. * Constraints on define_expand are never used, remove them (most other ports do not have constraints on define_expands either) * Some patterns have no name, makes debugging a bit harder * The divmodhi4 expander has been commented out since Day 1 of egcs. Explain why. Tested by building the compiler. For pdp11-elf, which is not a supported target according to backends.html, but it's not rejected. Not sure what the right target is to compile for. In any case, I obviously have no PDP-11 to test on. But the patch has no changes that impact code generation (the constraints were already ignored). OK for trunk? Ciao! Steven
RE: [patch] clean up pdp11.md a bit
These look fine. I'll defer to others on whether it should wait to Phase 1. I had tried to make divmod work but never figured out the reason why it did not. Thanks for answering that question. As for the subregs that Richard commented on -- I will gladly admit that this target isn't all clean yet. So that goes onto the list of things to work on, too. paul -Original Message- From: Steven Bosscher [mailto:stevenb@gmail.com] Sent: Tuesday, February 14, 2012 5:09 PM To: Koning, Paul; GCC Patches Subject: [patch] clean up pdp11.md a bit Hello, Just a few cleanups for things I noticed last weekend. * Constraints on define_expand are never used, remove them (most other ports do not have constraints on define_expands either) * Some patterns have no name, makes debugging a bit harder * The divmodhi4 expander has been commented out since Day 1 of egcs. Explain why. Tested by building the compiler. For pdp11-elf, which is not a supported target according to backends.html, but it's not rejected. Not sure what the right target is to compile for. In any case, I obviously have no PDP-11 to test on. But the patch has no changes that impact code generation (the constraints were already ignored). OK for trunk? Ciao! Steven
RE: [Patch,AVR] Print no-return functions as JMP
>There is no real post morten debugging on AVR as there is nothing like core >dump. But if there is any kind of debugging at all, you might use the debugger to put a breakpoint in abort(), and if so, having that invoked via jmp means you don't know who called it. So you'd want a way to suppress that optimization. paul
RE: [Patch,AVR] Print no-return functions as JMP
>> You should have a way to turn this off. Otherwise this makes >> debugging the call to abort impossible. > >What do you propose? > >o A command line option that is on per default like > -mnoreturn-tail-calls or -mjmp-noreturn > >o Hard-coded factor out some function names like "abort", > "exit", "_exit" I'd suggest the first option. That way you can do this for other similar functions like panic(). paul
RE: Commit: RX: Disable extender peepholes at -O3
> I am checking in the patch below to the mainline and 4.6 branch. It > disables the peephole optimizations in the rx.md file that combine a > load followed by a zero- or sign- extend operation. The disabling > only happens at -O3 (or higher) as although the peepholes reduce the > number of instructions they can introduce pipeline stalls that > actually make the program slower. (This is certainly true for the > coremark test). Doesn't that mean it should be disabled for any optimization other than optimize_size? It seems surprising to have a "size but not speed" optimization done at -O2. paul
