Re: New dump infrastructure
On Wed, Oct 17, 2012 at 6:55 PM, Xinliang David Li wrote:
> On Wed, Oct 17, 2012 at 2:08 AM, Richard Biener
> wrote:
>> On Wed, Oct 17, 2012 at 9:05 AM, Xinliang David Li
>> wrote:
>>> A more simpler use model is not to guard the dump statement at all --
>>> just express the intention a) what to dump; b) as what kind or to
>>> where
>>>
>>>
>>> 1) I want to dump the something as optimized message:
>>>
>>> dump_printf (MSG_OPTIMIZED, "blah...")
>>>
>>> dump_printf_loc (MSG_OPTIMIZED, "blah")
>>>
>>> 2) I want to dump something together with tree dump regardless of the flag:
>>>
>>>dump_printf (TDF_TREE, ...);
>>>
>>> 3) I want to dump something with tree dump when detailed flags is set:
>>>
>>> dump_printf (TDF_DETAILS, ...);
>>>
>>>
>>> The dumping code is not in the critical path, so the compile time
>>> should not be a concern.
>>
>> That's not true in all cases which is why I asked for the predicate to be
>> available, especially for the case where it guards multiple dump_* statement.
>>
>> Look for example at CCPs ccp_visit_stmt which calls
>>
>> if (dump_file && (dump_flags & TDF_DETAILS))
>> {
>> fprintf (dump_file, "\nVisiting statement:\n");
>> print_gimple_stmt (dump_file, stmt, 0, dump_flags);
>> }
>>
>
> Yes -- that means guarding is for performance reason and can be made optional.
>
>> and ends up calling evaluate_stmt most of the time:
>
> That looks too expensive even with the guard. Can it be turned off in
> non debug build?
>
>>
>> if (dump_file && (dump_flags & TDF_DETAILS))
>> {
>> fprintf (dump_file, "which is likely ");
>> switch (likelyvalue)
>> {
>> case CONSTANT:
>> fprintf (dump_file, "CONSTANT");
>> break;
>> case UNDEFINED:
>> fprintf (dump_file, "UNDEFINED");
>> break;
>> case VARYING:
>> fprintf (dump_file, "VARYING");
>> break;
>> default:;
>> }
>> fprintf (dump_file, "\n");
>> }
>>
>> such code is not something you'd want to do unconditionally.
>>
>
> ok.
>
>
>> I agree the use of the predicate can be reduced in some cases but it has
>> to be available for cases like the above. And eventually have a fast-path
>> inlined like
>>
>> inline bool dump_kind_p (int flags)
>> {
>> return any_dump_enabled_p ? dump_kind_p_1 (flags) : false;
>> }
>>
>> or a new inline function
>>
>> inline bool dump_enabled_p ()
>> {
>> return any_dump_enabled_p;
>> }
>>
>> at the cost of one extra global flag we'd need to maintain. Probably the
>> latter would be what you prefer? That way we don't get redundant
>> or even conflicting flags on the predicate check vs. the dump stmts.
>>
>
> Another way:
>
> To make the dumping code as concise as possible, we can make
> dump_printf a inlinable wrapper:
>
> inline void dump_printf (...)
> {
>if (!any_dump_enabled_p)
> return;
>
> // regular stuff
> }
You still have the issue that // regular stuff may appear to possibly
clobber any_dump_enabled_p and thus repeated any_dump_enabled_p
checks are not optimized by the compiler (we don't have predicated
value-numbering (yet)).
So I prefer the guard. I suppose after this discussion I prefer
a any_dump_enabled_p () guard instead of one with (repeated) flags.
Richard.
> thanks,
>
> David
>
>
>> Thanks,
>> Richard.
>>
>>> David
>>>
>>>
>>> On Tue, Oct 16, 2012 at 11:42 PM, Sharad Singhai wrote:
> 1. OK, I understand that e.g.
>
> if (dump_file && (dump_flags & TDF_DETAILS))
>
>should be converted into:
>
> if (dump_kind_p (TDF_DETAILS))
>
>But what about current code that does not care about dump_flags?
>E.g. converting simple
>
> if (dump_file)
>
>to
>
> if (dump_kind_p (0))
>
>won't work, dump_kind_p will always return zero in such cases.
Yes, you are right, the conversion is not completely mechanical and
some care must be taken to preserve the original intent. I think one
of the following might work in the case where a pass doesn't care
about the dump_flags
1. use generic pass type flags, such as TDF_TREE, TDF_IPA, TDF_RTL
which are guaranteed to be set depending on the pass type,
2. this dump statement might be a better fit for MSG_* flags if it
deals with optimizations. Sometimes "if (dump_file) fpritnf
(dump_file, ...)" idiom was used for these situations and now these
sites might be perfect candidate for converting into MSG_* flags.
If a cleaner way to handle this is desired, perhaps we can add an
unconditional "dump_printf_always (...)", but currently it seems
unnecessary. Note that for optimization messages which should always
be printed, one can use MSG_ALL flag. However, no analogous flag
exists for regular dumps. How about adding a corresponding TDF_ALL
flag? Would that work?
>
>
> 2. d
Re: Bugzilla new bug page
On 11 May 2011 15:05, Jonathan Wakely wrote: > Can we increase the size of the text at the top of the Enter Bug page? > > "Before reporting a bug, please read the bug writing guidelines, > please look at the list of most frequently reported bugs, and please > search for the bug." > > It's not very prominent. Ping? Other bugzillas I've used have a big red text box that very prominently tells the submitted to search for existing bugs. We should use something like that to point to the guidelines and the "not a bug" docs. The current notice is almost invisible, so it's hardly a surprise noone bothers to read the guidelines. I'd do it myself but I don't know where the relevant pages are. Are the bugzilla templates in CVS, or only on the server?
Re: Bugzilla new bug page
Le 18. 10. 12 14:06, Jonathan Wakely a écrit : > Other bugzillas I've used have a big red text box that very > prominently tells the submitted to search for existing bugs. Do you have an example of such Bugzillas? Mozilla and RedHat have no such big red box. KDE has something closer. > I'd do it myself but I don't know where the relevant pages are. Are > the bugzilla templates in CVS, or only on the server? Only on the server.
Re: Bugzilla new bug page
On 18 October 2012 13:16, Frédéric Buclin wrote: > Le 18. 10. 12 14:06, Jonathan Wakely a écrit : >> Other bugzillas I've used have a big red text box that very >> prominently tells the submitted to search for existing bugs. > > Do you have an example of such Bugzillas? Mozilla and RedHat have no > such big red box. KDE has something closer. Yes, maybe I was thinking of KDE's "don't skip this step" and mis-remembering the details of how it looks. How it looks doesn't really matter, I just think the current notice at the top of the Enter Bug page is almost invisible and should be more prominent. Another option would be to add some of the first few sentences from http://gcc.gnu.org/bugs/ "Before you report a bug, please check the list of well-known bugs and, if possible, try a current release or development snapshot. Before reporting that GCC compiles your code incorrectly, compile it with gcc -Wall -Wextra and see whether this shows anything wrong with your code. Similarly, if compiling with -fno-strict-aliasing -fwrapv makes a difference, your code probably is not correct. See http://gcc.gnu.org/bugs/ for the full bug writing guidelines" I'll prepare some mockups for people to look at and see if they like it.
Re: Bugzilla new bug page
Le 18. 10. 12 14:27, Jonathan Wakely a écrit : > I'll prepare some mockups for people to look at and see if they like it. Please file a bug and CC me. It's much easier to track progress in Bugzilla than per email. LpSolit
Re: Bugzilla new bug page
On 18 October 2012 13:32, Frédéric Buclin wrote: > Le 18. 10. 12 14:27, Jonathan Wakely a écrit : >> I'll prepare some mockups for people to look at and see if they like it. > > Please file a bug and CC me. It's much easier to track progress in > Bugzilla than per email. OK, thanks, it's now PR 54973
cse_process_notes_1 issue ?
Hi,
I'm trying to track a bug down on a private backend which occurs during
CSE pass (gcc-4.6.3, pr27364.c).
In the following RTL, the hardware (reg:HI r2), whose natural mode is
HImode, is set to 0, but when analysing the REG_EQUAL notes of the MULT
insn during CSE pass, the (reg:SI r2) is computed to be equivalent to 0,
which is wrong (the target is big endian).
(insn 51 9 52 3 (set (reg:HI 2 r2)
(const_int 0 [0])) gcc.c-torture/execute/pr27364.c:5 18 {*movhi1}
(expr_list:REG_DEAD (reg:HI 31)
(expr_list:REG_EQUAL (const_int 0 [0])
(nil
(insn 52 51 12 3 (set (reg:HI 3 r3 [orig:2+2 ] [2])
(reg/v:HI 20 [ number_of_digits_to_use ]))
gcc.c-torture/execute/pr27364.c:5 18 {*movhi1}
(expr_list:REG_DEAD (reg/v:HI 20 [ number_of_digits_to_use ])
(nil)))
(insn 12 52 13 3 (set (reg:SI 0 r0)
(const_int 3321928 [0x32b048]))
gcc.c-torture/execute/pr27364.c:5 19 {movsi}
(nil))
(insn 13 12 16 3 (parallel [
(set (reg:SI 0 r0)
(mult:SI (reg:SI 2 r2)
(reg:SI 0 r0)))
(clobber (reg:SI 2 r2))
]) gcc.c-torture/execute/pr27364.c:5 54 {*mulsi3_call}
(expr_list:REG_EQUAL (mult:SI (reg:SI 2 r2)
(const_int 3321928 [0x32b048]))
(expr_list:REG_DEAD (reg:HI 3 r3)
(expr_list:REG_UNUSED (reg:SI 2 r2)
(nil)
I think a mode size check is missing when processing REG code in
cse_process_notes_1. Adding such a check prevents the CSE pass from
elimintating the MULT instruction.
But then this MULT insn is simplified during the combine pass:
Trying 12 -> 13:
...
Successfully matched this instruction:
(set (reg:SI 0 r0)
(const_int 0 [0]))
deferring deletion of insn with uid = 12.
deferring deletion of insn with uid = 52.
modifying insn i313 r0:SI=0
deferring rescan insn with uid = 13.
So double middle-end bug or do I miss something?
Thanks,
Aurelien
Inconsistency between code and docs
Hello, I have found a strange inconsistency between code and docs for TARGET_ASM_NAMED_SECTION. Docs say (http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gccint/File-Framework.html#index-TARGET_005fASM_005fNAMED_005fSECTION-4472): " If decl is non-NULL, it is the VAR_DECL or FUNCTION_DECL with which this section is associated." However, in dwarf2out.c, line 21030 we have the following code: tree comdat_key = get_identifier (ref->info); /* Terminate the previous .debug_macinfo section. */ dw2_asm_output_data (1, 0, "End compilation unit"); targetm.asm_out.named_section (DEBUG_MACRO_SECTION, SECTION_DEBUG | SECTION_LINKONCE, comdat_key); This passes comdat_key to named_section hook which expected either a VAR_DECL or a FUNCTION_DECL. I noticed as I am using DECL_ATTRIBUTES inside the hook on decl and it failed with checking enabled as an IDENTIFIER_NODE (as passed by dwarf2out.c) is not a DECL. It seems to me to be a bug of dwarf2out.c since it should maybe generate a VAR_DECL instead of an IDENTIFIER_NODE. Is this the case? Paulo Matos
Re: New dump infrastructure
sounds good.
thanks,
David
On Thu, Oct 18, 2012 at 1:52 AM, Richard Biener
wrote:
> On Wed, Oct 17, 2012 at 6:55 PM, Xinliang David Li wrote:
>> On Wed, Oct 17, 2012 at 2:08 AM, Richard Biener
>> wrote:
>>> On Wed, Oct 17, 2012 at 9:05 AM, Xinliang David Li
>>> wrote:
A more simpler use model is not to guard the dump statement at all --
just express the intention a) what to dump; b) as what kind or to
where
1) I want to dump the something as optimized message:
dump_printf (MSG_OPTIMIZED, "blah...")
dump_printf_loc (MSG_OPTIMIZED, "blah")
2) I want to dump something together with tree dump regardless of the flag:
dump_printf (TDF_TREE, ...);
3) I want to dump something with tree dump when detailed flags is set:
dump_printf (TDF_DETAILS, ...);
The dumping code is not in the critical path, so the compile time
should not be a concern.
>>>
>>> That's not true in all cases which is why I asked for the predicate to be
>>> available, especially for the case where it guards multiple dump_*
>>> statement.
>>>
>>> Look for example at CCPs ccp_visit_stmt which calls
>>>
>>> if (dump_file && (dump_flags & TDF_DETAILS))
>>> {
>>> fprintf (dump_file, "\nVisiting statement:\n");
>>> print_gimple_stmt (dump_file, stmt, 0, dump_flags);
>>> }
>>>
>>
>> Yes -- that means guarding is for performance reason and can be made
>> optional.
>>
>>> and ends up calling evaluate_stmt most of the time:
>>
>> That looks too expensive even with the guard. Can it be turned off in
>> non debug build?
>>
>>>
>>> if (dump_file && (dump_flags & TDF_DETAILS))
>>> {
>>> fprintf (dump_file, "which is likely ");
>>> switch (likelyvalue)
>>> {
>>> case CONSTANT:
>>> fprintf (dump_file, "CONSTANT");
>>> break;
>>> case UNDEFINED:
>>> fprintf (dump_file, "UNDEFINED");
>>> break;
>>> case VARYING:
>>> fprintf (dump_file, "VARYING");
>>> break;
>>> default:;
>>> }
>>> fprintf (dump_file, "\n");
>>> }
>>>
>>> such code is not something you'd want to do unconditionally.
>>>
>>
>> ok.
>>
>>
>>> I agree the use of the predicate can be reduced in some cases but it has
>>> to be available for cases like the above. And eventually have a fast-path
>>> inlined like
>>>
>>> inline bool dump_kind_p (int flags)
>>> {
>>> return any_dump_enabled_p ? dump_kind_p_1 (flags) : false;
>>> }
>>>
>>> or a new inline function
>>>
>>> inline bool dump_enabled_p ()
>>> {
>>> return any_dump_enabled_p;
>>> }
>>>
>>> at the cost of one extra global flag we'd need to maintain. Probably the
>>> latter would be what you prefer? That way we don't get redundant
>>> or even conflicting flags on the predicate check vs. the dump stmts.
>>>
>>
>> Another way:
>>
>> To make the dumping code as concise as possible, we can make
>> dump_printf a inlinable wrapper:
>>
>> inline void dump_printf (...)
>> {
>>if (!any_dump_enabled_p)
>> return;
>>
>> // regular stuff
>> }
>
> You still have the issue that // regular stuff may appear to possibly
> clobber any_dump_enabled_p and thus repeated any_dump_enabled_p
> checks are not optimized by the compiler (we don't have predicated
> value-numbering (yet)).
>
> So I prefer the guard. I suppose after this discussion I prefer
> a any_dump_enabled_p () guard instead of one with (repeated) flags.
>
> Richard.
>
>> thanks,
>>
>> David
>>
>>
>>> Thanks,
>>> Richard.
>>>
David
On Tue, Oct 16, 2012 at 11:42 PM, Sharad Singhai
wrote:
>> 1. OK, I understand that e.g.
>>
>> if (dump_file && (dump_flags & TDF_DETAILS))
>>
>>should be converted into:
>>
>> if (dump_kind_p (TDF_DETAILS))
>>
>>But what about current code that does not care about dump_flags?
>>E.g. converting simple
>>
>> if (dump_file)
>>
>>to
>>
>> if (dump_kind_p (0))
>>
>>won't work, dump_kind_p will always return zero in such cases.
>
>
> Yes, you are right, the conversion is not completely mechanical and
> some care must be taken to preserve the original intent. I think one
> of the following might work in the case where a pass doesn't care
> about the dump_flags
>
> 1. use generic pass type flags, such as TDF_TREE, TDF_IPA, TDF_RTL
> which are guaranteed to be set depending on the pass type,
> 2. this dump statement might be a better fit for MSG_* flags if it
> deals with optimizations. Sometimes "if (dump_file) fpritnf
> (dump_file, ...)" idiom was used for these situations and now these
> sites might be perfect candidate for converting into MSG_* flags.
>
> If a cleaner way to handle this is desired, perhaps we can add an
> unconditional "dump_printf_always (...)", but currently it seems
Re: Bugzilla new bug page
On 18 October 2012 13:43, Jonathan Wakely wrote: > On 18 October 2012 13:32, Frédéric Buclin wrote: >> Le 18. 10. 12 14:27, Jonathan Wakely a écrit : >>> I'll prepare some mockups for people to look at and see if they like it. >> >> Please file a bug and CC me. It's much easier to track progress in >> Bugzilla than per email. > > OK, thanks, it's now PR 54973 That PR now has a link to a mocked up bugzilla page: http://www.kayari.plus.com/gcc/enter_bug.cgi-1.html which I think would be a significant improvement, without getting in the way or being an eyesore. Do any other maintainers have an opinion on it? Both times I've suggested this there's been a deafening silence from other maintainers and regular bugzilla triagers. Is everyone else happy with the status quo of asking many new bug reporters to provide the information requested by the (not very prominent) notice? I find it quite frustrating having to keep repeating the same requests when triaging bugs, but it's wrong to blame reporters for not following a link that's barely visible.
Re: New dump infrastructure
> You still have the issue that // regular stuff may appear to possibly > clobber any_dump_enabled_p and thus repeated any_dump_enabled_p > checks are not optimized by the compiler (we don't have predicated > value-numbering (yet)). > So I prefer the guard. I suppose after this discussion I prefer > a any_dump_enabled_p () guard instead of one with (repeated) flags. I have updated 'dump_enabled_p ()' a little bit in http://gcc.gnu.org/ml/gcc-patches/2012-10/msg01690.html to avoid checking of flags. If this looks better, I can update the guard conditions in vectorization passes from 'if (dump_kind_p (...))' to 'if (dump_enabled_p ())'. Thanks, Sharad
Re: Bugzilla new bug page
On Thu, Oct 18, 2012 at 10:14 AM, Jonathan Wakely wrote: > > That PR now has a link to a mocked up bugzilla page: > http://www.kayari.plus.com/gcc/enter_bug.cgi-1.html which I think > would be a significant improvement, without getting in the way or > being an eyesore. > > Do any other maintainers have an opinion on it? Looks good to me. Thanks. Ian
Re: Inconsistency between code and docs
On Thu, Oct 18, 2012 at 8:02 AM, Paulo Matos wrote: > > I have found a strange inconsistency between code and docs for > TARGET_ASM_NAMED_SECTION. > > Docs say > (http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gccint/File-Framework.html#index-TARGET_005fASM_005fNAMED_005fSECTION-4472): > " If decl is non-NULL, it is the VAR_DECL or FUNCTION_DECL with which this > section is associated." > > However, in dwarf2out.c, line 21030 we have the following code: > tree comdat_key = get_identifier (ref->info); > /* Terminate the previous .debug_macinfo section. */ > dw2_asm_output_data (1, 0, "End compilation unit"); > targetm.asm_out.named_section (DEBUG_MACRO_SECTION, > SECTION_DEBUG > | SECTION_LINKONCE, > comdat_key); > > This passes comdat_key to named_section hook which expected either a VAR_DECL > or a FUNCTION_DECL. > I noticed as I am using DECL_ATTRIBUTES inside the hook on decl and it failed > with checking enabled as an IDENTIFIER_NODE (as passed by dwarf2out.c) is not > a DECL. > > It seems to me to be a bug of dwarf2out.c since it should maybe generate a > VAR_DECL instead of an IDENTIFIER_NODE. Is this the case? I think the bug is in the documentation, and that TARGET_ASM_NAMED_SECTION should accept an IDENTIFIER_NODE. Ian
testing that a Gimple call argument is a string...
Hello
I'm coding in MELT the ex06/ of https://github.com/bstarynk/melt-examples/
which should typecheck calls to variadic functions json_pack & json_unpack
from http://www.digip.org/jansson (a JSON library in C).
I'm working on a MELT pass on Gimple/SSA after phiopt (maybe that place is
wrong?)
I don't understand well how to check that a given Gimple argument is a string.
I was thinking of using useless_type_conversion_p or types_compatible_p on the
TREE_TYPE of some POINTER_TYPE with char_type_node, but it seems to not work as
I would expect
How would you (in C++ or C for a 4.6) code such a test (that argument 2 of some
GIMPLE_CALL is a string, ie. a char* in C parlance)?
(I'm coding in MELT, but I am not asking a MELT specific question; I have hard
time understanding how that should be coded in C++).
Or where is the typechecking for __attribute__((format(printf))) functions done
in the GCC source tree?
Regards.
--
Basile STARYNKEVITCH http://starynkevitch.net/Basile/
email: basilestarynkevitchnet mobile: +33 6 8501 2359
8, rue de la Faiencerie, 92340 Bourg La Reine, France
*** opinions {are only mines, sont seulement les miennes} ***
Re: testing that a Gimple call argument is a string...
On Thu, 2012-10-18 at 22:33 +0200, Basile Starynkevitch wrote: > Hello > > I'm coding in MELT the ex06/ of https://github.com/bstarynk/melt-examples/ > which should typecheck calls to variadic functions json_pack & json_unpack > from http://www.digip.org/jansson (a JSON library in C). > > I'm working on a MELT pass on Gimple/SSA after phiopt (maybe that place is > wrong?) > > I don't understand well how to check that a given Gimple argument is a string. > I was thinking of using useless_type_conversion_p or types_compatible_p on > the TREE_TYPE of some POINTER_TYPE with char_type_node, but it seems to not > work as I would expect > > How would you (in C++ or C for a 4.6) code such a test (that argument 2 of > some GIMPLE_CALL is a string, ie. a char* in C parlance)? > > (I'm coding in MELT, but I am not asking a MELT specific question; I have > hard time understanding how that should be coded in C++). FWIW I've written a type-checker for variadic arguments in my Python plugin; this probably won't help you but the python code can be seen here: http://git.fedorahosted.org/cgit/gcc-python-plugin.git/tree/libcpychecker/formatstrings.py (it checks usage of the CPython API) In theory I'm looking up the "char" type via integer_types[itk_char], then getting the pointer type via build_pointer_type() to get "char *", then comparing for pointer equality. But it got a *lot* more complicated than that when handling special cases (e.g. a typedef that's a char is also acceptable). > Or where is the typechecking for __attribute__((format(printf))) functions > done in the GCC source tree? I believe it's within gcc/c-family/c-format.c Hope this is helpful Dave
Re: Bugzilla new bug page
Jonathan Wakely schrieb: On 18 October 2012 13:43, Jonathan Wakely wrote: On 18 October 2012 13:32, Frédéric Buclin wrote: Le 18. 10. 12 14:27, Jonathan Wakely a écrit : I'll prepare some mockups for people to look at and see if they like it. Please file a bug and CC me. It's much easier to track progress in Bugzilla than per email. OK, thanks, it's now PR 54973 That PR now has a link to a mocked up bugzilla page: http://www.kayari.plus.com/gcc/enter_bug.cgi-1.html which I think would be a significant improvement, without getting in the way or being an eyesore. As you are improving the bug reporting documentation: http://gcc.gnu.org/bugs/ does not explain how to report a bug that only occurs with LTO, i.e. is triggered by dozens of sources compiled with -flto but does not occur if compiled / "linked" without LTO. Posting dozens of C files? Or s files? Or an archive? What format? Johann
Re: Bugzilla new bug page
On 18 October 2012 23:08, Georg-Johann Lay wrote: > > As you are improving the bug reporting documentation: I'm not changing the documentation. I'm just trying to make some existing text harder to ignore. > http://gcc.gnu.org/bugs/ > > does not explain how to report a bug that only occurs with LTO, i.e. > is triggered by dozens of sources compiled with -flto but does not > occur if compiled / "linked" without LTO. > > Posting dozens of C files? Or s files? Or an archive? What format? I don't know how to answer those questions so I'm not going to change that.
