Re: [Patch, Fortran] PR56907 - do not 'pack' arrays passed to C_LOC
>> What I don't quite understand is:
>> @@ -6317,8 +6317,13 @@ conv_isocbinding_function (gfc_se *se, gfc_expr
>> *expr)
>> {
>> if (arg->expr->rank == 0)
>> gfc_conv_expr_reference (se, arg->expr);
>> - else
>> + else if (gfc_is_simply_contiguous (arg->expr, false))
>> gfc_conv_array_parameter (se, arg->expr, true, NULL, NULL, NULL);
>> + else
>> +{
>> + gfc_conv_expr_descriptor (se, arg->expr);
>> + se->expr = gfc_conv_descriptor_data_get (se->expr);
>> +}
>>
>>
>> Why doesn't 'gfc_conv_array_parameter' handle this situation properly?
>
> Well, it does: As it doesn't know whether the array is contiguous or not -
> it packs the array. That's what one usually wants.
>
> However, for C_LOC one knows that the array is contiguous - the user
> promises this to the compiler - and, hence, no packing is needed.
Ok, I see. Then the patch is certainly ok. (The fact that
conv_isocbinding_function is used also for C_ASSOCIATED and C_FUNLOC
is no problem either, I guess).
However, if calling C_LOC on a non-contiguous array is invalid,
shouldn't one add a check for cases like
integer, dimension(1:5,1:5), target :: zzz
type(c_ptr) :: ptr
ptr = c_loc (zzz(4:,4:))
where the compiler can easily tell that the argument is not contiguous ... ?
Cheers,
Janus
Re: [Patch, Fortran] PR56907 - do not 'pack' arrays passed to C_LOC
Am 20.04.2013 12:42, schrieb Janus Weil: Well, it does: As it doesn't know whether the array is contiguous or not - it packs the array. That's what one usually wants. However, for C_LOC one knows that the array is contiguous - the user promises this to the compiler - and, hence, no packing is needed. Ok, I see. Then the patch is certainly ok. (The fact that conv_isocbinding_function is used also for C_ASSOCIATED and C_FUNLOC is no problem either, I guess). Well, the code in question is under: if (expr->value.function.isym->id == GFC_ISYM_C_LOC) However, if calling C_LOC on a non-contiguous array is invalid, shouldn't one add a check for cases like integer, dimension(1:5,1:5), target :: zzz type(c_ptr) :: ptr ptr = c_loc (zzz(4:,4:)) where the compiler can easily tell that the argument is not contiguous ... ? Definitely. I think there is also a PR about adding a gfc_is_simply_noncontiguous() or something like that. It has several uses: C_LOC, pointer-assignment to a contiguous pointer, removing some "if"s related to packing (as one knows that internal_pack will pack). And for compile-time simplification of the (unimplemented) Fortran 2008 function "is_contiguous". Tobias
Re: [Patch, Fortran] PR56907 - do not 'pack' arrays passed to C_LOC
>>> Well, it does: As it doesn't know whether the array is contiguous or not >>> - it packs the array. That's what one usually wants. However, for C_LOC one >>> knows that the array is contiguous - the user promises this to the compiler >>> - and, hence, no packing is needed. >> >> Ok, I see. Then the patch is certainly ok. (The fact that >> conv_isocbinding_function is used also for C_ASSOCIATED and C_FUNLOC >> is no problem either, I guess). > > Well, the code in question is under: > > if (expr->value.function.isym->id == GFC_ISYM_C_LOC) Ah, sure, I missed that. Btw, wouldn't it make more sense to split up 'conv_isocbinding_function' into three separate functions, since there isn't really any common code and one could directly call them in: case GFC_ISYM_C_ASSOCIATED: case GFC_ISYM_C_FUNLOC: case GFC_ISYM_C_LOC: conv_isocbinding_function (se, expr); break; Anyway, the patch is ok as is (or with this additional cleanup, if you prefer). >> However, if calling C_LOC on a non-contiguous array is invalid, >> shouldn't one add a check for cases like >>integer, dimension(1:5,1:5), target :: zzz >>type(c_ptr) :: ptr >>ptr = c_loc (zzz(4:,4:)) >> where the compiler can easily tell that the argument is not contiguous ... >> ? > > Definitely. I think there is also a PR about adding a > gfc_is_simply_noncontiguous() or something like that. It has several uses: > C_LOC, pointer-assignment to a contiguous pointer, removing some "if"s > related to packing (as one knows that internal_pack will pack). And for > compile-time simplification of the (unimplemented) Fortran 2008 function > "is_contiguous". Right: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45424 Cheers, Janus
Re: [patch][mips] split mips_reorg in pre- and post-dbr_schedule parts
*ping* MIPS maintainers... On Mon, Apr 15, 2013 at 5:09 PM, Jeff Law wrote: > On 04/14/2013 08:20 AM, Steven Bosscher wrote: >> >> Hello, >> >> This patch splits mips_reorg.c in a pre-dbr_schedule part and a new, >> machine specific post-dbr_schedule pass. With this patch, >> cleanup_barriers and dbr_schedule can be static functions again. >> >> Cross-built&tested mips-sim. OK for trunk? >> >> Ciao! >> Steven >> >> >> mips_post_dbr_reorg_as_machine_pass.diff.txt >> >> >> * config/mips/mips.c: Include tree-pass.h. >> (mips_reorg): Split in pre- and post-dbr_schedule parts. >> (mips_machine_reorg2): Move mips_reorg post-dbr_schedule parts >> here. >> (pass_mips_machine_reorg2): New machine specific pass. >> (insert_pass_mips_machine_reorg2): New pass plugin definition. >> (mips_option_override): Register the new pass. >> * rtl.h (cleanup_barriers): Remove prototype. >> (dbr_schedule): Likewise. >> * jump.c (cleanup_barriers): Make static. >> * reorg.c (dbr_schedule): Likewise. > > The rtl, jump & reorg bits are fine with me. I don't know enough about the > MIPS specific bits to comment on them in any meaningful way. > > jeff >
Re: [PATCH] Fix PR56982, handle setjmp like non-local labels
Richard Biener writes:
> PR tree-optimization/56982
> * builtins.def (BUILT_IN_LONGJMP): longjmp is not a leaf
> function.
> * gimplify.c (gimplify_call_expr): Notice special calls.
> (gimplify_modify_expr): Likewise.
> * tree-cfg.c (make_abnormal_goto_edges): Handle setjmp-like
> abnormal control flow receivers.
> (call_can_make_abnormal_goto): Handle cfun->calls_setjmp
> in the same way as cfun->has_nonlocal_labels.
> (gimple_purge_dead_abnormal_call_edges): Likewise.
> (stmt_starts_bb_p): Make setjmp-like abnormal control flow
> receivers start a basic-block.
This breaks libgo.
../../../gcc/libgo/runtime/proc.c: In function ‘runtime_mcall.constprop.7’:
../../../gcc/libgo/runtime/proc.c:419:13: error: ‘({anonymous})’ may be used
uninitialized in this function [-Werror=maybe-uninitialized]
getcontext(&gp->context);
^
Andreas.
--
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
Re: [patch, fortran, backport, 4.8] PR51825 - Fortran runtime error: Cannot match namelist object name
On Sat, 13 Apr 2013 10:29:26 +0200, Janus Weil wrote: Hi Tilo, I would like to backport the fix for PR51825 I posted here http://gcc.gnu.org/ml/gcc-patches/2013-03/msg00316.html to the 4.8 branch. well, the usual gfortran policy is to only do backports of regression fixes. In exceptional cases, also non-regression fixes can be backported (and have been in the past), if the bug is extremely severe and/or the the fix is extremely simple (IMHO the most severe type of bug is a wrong-code issue, where the user does not see any kind of error message, but just 'silently' gets wrong results). Hi Janus, the bug is indeed pretty nasty. What happens is, that any namelist line involving an array index and nested derived types like this arr(2)%a%b = 1 completely ignores the given array index and silently reads it as 1. So no matter what you write as index > 1 in a line like arr()%a%b = 1 libgfortran always silently reads the index == 1 as arr(1)%a%b = 1 and sets the first element of arr instead of setting arr(index). When I hit this (as a gfortran user) it took me quite a while to figure out whats going on. What makes it even more nasty: This arr(2)%b = 1 did work (only one level of derived type given). I'm personally not too affected anymore, because this bug made me writing my first libgfortran patch ever and since then I use gfortran trunk anyway ;-) But for others the bug is "sort of mean" ... Regards, Tilo
[patch] fix anachronism in libstdc+ docs
* doc/xml/manual/extensions.xml: Fix anachronism. committed to trunk commit ab11f53f4046adc88e1f43568836c572bcd3bc44 Author: Jonathan Wakely Date: Sat Apr 20 20:31:32 2013 +0100 * doc/xml/manual/extensions.xml: Fix anachronism. diff --git a/libstdc++-v3/doc/xml/manual/extensions.xml b/libstdc++-v3/doc/xml/manual/extensions.xml index 1f3da2f..7b35d05 100644 --- a/libstdc++-v3/doc/xml/manual/extensions.xml +++ b/libstdc++-v3/doc/xml/manual/extensions.xml @@ -87,10 +87,11 @@ extensions, be aware of two things: 3.1, 3.2 and 3.3). - Please note that the upcoming C++ standard has first-class + Please note that the concept checks only validate the requirements + of the old C++03 standard. C++11 was expected to have first-class support for template parameter constraints based on concepts in the core - language. This will obviate the need for the library-simulated concept - checking described above. + language. This would have obviated the need for the library-simulated concept + checking described above, but was not part of C++11.
Re: [PATCH] Fix PR56982, handle setjmp like non-local labels
On Sat, Apr 20, 2013 at 5:14 AM, Andreas Schwab wrote:
> Richard Biener writes:
>
>> PR tree-optimization/56982
>> * builtins.def (BUILT_IN_LONGJMP): longjmp is not a leaf
>> function.
>> * gimplify.c (gimplify_call_expr): Notice special calls.
>> (gimplify_modify_expr): Likewise.
>> * tree-cfg.c (make_abnormal_goto_edges): Handle setjmp-like
>> abnormal control flow receivers.
>> (call_can_make_abnormal_goto): Handle cfun->calls_setjmp
>> in the same way as cfun->has_nonlocal_labels.
>> (gimple_purge_dead_abnormal_call_edges): Likewise.
>> (stmt_starts_bb_p): Make setjmp-like abnormal control flow
>> receivers start a basic-block.
>
> This breaks libgo.
>
> ../../../gcc/libgo/runtime/proc.c: In function ‘runtime_mcall.constprop.7’:
> ../../../gcc/libgo/runtime/proc.c:419:13: error: ‘({anonymous})’ may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
>getcontext(&gp->context);
> ^
That's an unusually useless error message. There are no local structs
in this function. Without digging into the compiler I'm not sure what
this means or how to fix it.
Ian
Simple change in dot dumper -- Display profile count and branch probability
Hi, the graph dump file currently does not show any profile
information. The following simple patch fixed that. Ok for trunk?
thanks,
David
2013-04-20 Xinliang David Li
* graph.c (draw_cfg_node): Add count and frequency info.
(draw_cfg_node_succ_edges): Add branch probility as label.
Index: graph.c
===
--- graph.c (revision 198108)
+++ graph.c (working copy)
@@ -110,6 +110,9 @@ draw_cfg_node (pretty_printer *pp, int f
else
{
pp_character (pp, '{');
+ if (bb->count)
+ pp_printf (pp, "COUNT:" HOST_WIDEST_INT_PRINT_DEC, bb->count);
+ pp_printf (pp, " FREQ:%i |", bb->frequency);
pp_write_text_to_stream (pp);
dump_bb_for_graph (pp, bb);
pp_character (pp, '}');
@@ -155,11 +158,12 @@ draw_cfg_node_succ_edges (pretty_printer
pp_printf (pp,
"\tfn_%d_basic_block_%d:s -> fn_%d_basic_block_%d:n "
-"[style=%s,color=%s,weight=%d,constraint=%s];\n",
+"[style=%s,color=%s,weight=%d,constraint=%s,
label=\"[%i%%]\"];\n",
funcdef_no, e->src->index,
funcdef_no, e->dest->index,
style, color, weight,
-(e->flags & (EDGE_FAKE | EDGE_DFS_BACK)) ? "false" : "true");
+(e->flags & (EDGE_FAKE | EDGE_DFS_BACK)) ? "false" : "true",
+e->probability * 100 / REG_BR_PROB_BASE);
}
pp_flush (pp);
}
Re: Simple change in dot dumper -- Display profile count and branch probability
On Sun, Apr 21, 2013 at 12:47 AM, Xinliang David Li wrote:
> Index: graph.c
> ===
> --- graph.c (revision 198108)
> +++ graph.c (working copy)
> @@ -110,6 +110,9 @@ draw_cfg_node (pretty_printer *pp, int f
>else
> {
>pp_character (pp, '{');
> + if (bb->count)
> + pp_printf (pp, "COUNT:" HOST_WIDEST_INT_PRINT_DEC, bb->count);
> + pp_printf (pp, " FREQ:%i |", bb->frequency);
>pp_write_text_to_stream (pp);
>dump_bb_for_graph (pp, bb);
>pp_character (pp, '}');
This doesn't belong here, please put it in cfghooks.c:dump_bb_for_graph.
Otherwise, looks good me.
Ciao!
Steven
Re: [google][4.7] Generate a label for the split cold function while using -freorder-blocks-and-partition
Fixed a bug in the previous patch sent, where I did not check if the
switch section was actually to the cold function part. Updated patch
attached.
Thanks
Sri
On Fri, Apr 19, 2013 at 1:58 PM, Sriraman Tallam wrote:
> Updated patch attached.
>
> Thanks
> Sri
>
> On Fri, Apr 19, 2013 at 1:43 PM, Sriraman Tallam wrote:
>> Hi,
>>
>> This patch generates labels for cold function parts that are split when
>> using the option -freorder-blocks-and-partition. The cold label name
>> is generated by suffixing ".cold" to the assembler name of the hot
>> function.
>>
>> This is useful when getting back traces from gdb when the cold function
>> part does get executed.
>>
>> I will port this patch to trunk, please let me know what you think.
>>
>> Thanks
>> Sri
Patch to generate labels for cold function parts that are split when
using the option -freorder-blocks-and-partition. The cold label name
is generated by suffixing ".cold" to the assembler name of the hot
function.
This is useful when getting back traces from gdb when the cold function
part does get executed.
Index: final.c
===
--- final.c (revision 198081)
+++ final.c (working copy)
@@ -1934,6 +1934,21 @@ final_scan_insn (rtx insn, FILE *file, int optimiz
targetm.asm_out.function_switched_text_sections (asm_out_file,
current_function_decl,
in_cold_section_p);
+ /* Emit a label for the split cold section. Form label name by
+suffixing ".cold" to the function's assembler name. */
+ if (in_cold_section_p)
+ {
+ char *cold_function_name;
+ const char *mangled_function_name;
+ tree asm_name = DECL_ASSEMBLER_NAME (current_function_decl);
+
+ mangled_function_name = IDENTIFIER_POINTER (asm_name);
+ cold_function_name = XNEWVEC (char,
+ strlen (mangled_function_name) + strlen (".cold") + 1);
+ sprintf (cold_function_name, "%s.cold", mangled_function_name);
+ ASM_OUTPUT_LABEL (asm_out_file, cold_function_name);
+ XDELETEVEC (cold_function_name);
+ }
break;
case NOTE_INSN_BASIC_BLOCK:
Index: testsuite/gcc.dg/tree-prof/cold_partition_label.c
===
--- testsuite/gcc.dg/tree-prof/cold_partition_label.c (revision 0)
+++ testsuite/gcc.dg/tree-prof/cold_partition_label.c (revision 0)
@@ -0,0 +1,39 @@
+/* Test case to check if function foo gets split and the cold function
+ gets a label. */
+/* { dg-require-effective-target freorder } */
+/* { dg-options "-O2 -freorder-blocks-and-partition --save-temps" } */
+
+#define SIZE 1
+
+const char *sarr[SIZE];
+const char *buf_hot;
+const char *buf_cold;
+
+__attribute__((noinline))
+void
+foo (int path)
+{
+ int i;
+ if (path)
+{
+ for (i = 0; i < SIZE; i++)
+ sarr[i] = buf_hot;
+}
+ else
+{
+ for (i = 0; i < SIZE; i++)
+ sarr[i] = buf_cold;
+}
+}
+
+int
+main (int argc, char *argv[])
+{
+ buf_hot = "hello";
+ buf_cold = "world";
+ foo (argc);
+ return 0;
+}
+
+/* { dg-final-use { scan-assembler "foo.cold" } } */
+/* { dg-final-use { cleanup-saved-temps } } */
Re: GCC does not support *mmintrin.h with function specific opts
Ping.
On Wed, Apr 17, 2013 at 7:13 PM, Sriraman Tallam wrote:
> +HJ
>
> On Tue, Apr 16, 2013 at 1:54 PM, Sriraman Tallam wrote:
>> Hi,
>>
>> I have attached an updated patch that addresses all the comments raised.
>>
>> On Fri, Apr 12, 2013 at 1:58 AM, Jakub Jelinek wrote:
>>> On Thu, Apr 11, 2013 at 12:05:41PM -0700, Sriraman Tallam wrote:
I have attached a patch that fixes this. I have added an option
"-mgenerate-builtins" that will do two things. It will define a macro
"__ALL_ISA__" which will expose the *intrin.h functions. It will also
expose all the target specific builtins. -mgenerate-builtins will not
affect code generation.
>>>
>>> 1) this shouldn't be an option, either it can be made to work reliably,
>>>then it should be done always, or it can't, then it shouldn't be done
>>
>> Ok, it is on by default now. There is a way to turn it off, with
>> -mno-generate-builtins.
>>
>>> 2) have you verified that if you always generate all builtins, that the
>>>builtins not supported by the ISA selected from the command line are
>>>created with the right vector modes?
>>
>> This issue does not arise. When the target builtin is expanded, it is
>> checked if the ISA support is there, either via function specific
>> target opts or global target opts. If not, an error is raised. Test
>> case added for this, please see intrinsic_4.c in patch.
>>
>>> 3) the *intrin.h headers in the case where the guarding macro isn't defined
>>>should be surrounded by something like
>>>#ifndef __FMA4__
>>>#pragma GCC push options
>>>#pragma GCC target("fma4")
>>>#endif
>>>...
>>>#ifndef __FMA4__
>>>#pragma GCC pop options
>>>#endif
>>>so that everything that is in the headers is compiled with the ISA
>>>in question
>>
>> I do not think this should be done because it will break the inlining
>> ability of the header function and cause issues if the caller does not
>> specify the required ISA. The fact that the header functions are
>> marked extern __inline, with gnu_inline guarantees that a body will
>> not be generated and they will be inlined. If the caller does not
>> have the required ISA, appropriate errors will be raised. Test cases
>> added, see intrinsics_1.c, intrinsics_2.c
>>
>>> 4) what happens if you use the various vector types typedefed in the
>>>*intrin.h headers in code that doesn't support those ISAs? As TYPE_MODE
>>>for VECTOR_TYPE is a function call, perhaps it will just be handled as
>>>generic BLKmode vectors, which is desirable I think
>>
>> I checked some tests here. With -mno-sse for instance, vector types
>> are not permitted in function arguments and return values and gcc
>> raises a warning/error in each case. With return values, gcc always
>> gives an error if a SSE register is required in a return value. I
>> even fixed this message to not do it for functions marked as extern
>> inline, with "gnu_inline" keyword as a body for them will not be
>> generated.
>>
>>
>>> 5) what happens if you use a target builtin in a function not supporting
>>>the corresponding ISA, do you get proper error explaining what you are
>>>doing wrong?
>>
>> Yes, please sse intrinsic_4.c test in patch.
>>
>>> 6) what happens if you use some intrinsics in a function not supporting
>>>the corresponding ISA? Dunno if the inliner chooses not to inline it
>>>and error out because it is always_inline, or what exactly will happen
>>>then
>>
>> Same deal here. The intrinsic function will, guaranteed, to be inlined
>> into the caller which will be a corresponding builtin call. That
>> builtin call will trigger an error if the ISA is not supported.
>>
>> Thanks
>> Sri
>>
>>>
>>> For all this you certainly need testcases.
>>>
>>> Jakub
Re: Simple change in dot dumper -- Display profile count and branch probability
Thanks. The patch is revised.
David
2013-04-20 Xinliang David Li
* graph.c (draw_cfg_node_succ_edges): Add branch probility as label.
* cfghhooks.c (dump_bb_for_graph): Dump profile count and frquency.
* Makefile.in: New dependency.
Index: cfghooks.c
===
--- cfghooks.c (revision 198108)
+++ cfghooks.c (working copy)
@@ -30,6 +30,7 @@ along with GCC; see the file COPYING3.
#include "timevar.h"
#include "diagnostic-core.h"
#include "cfgloop.h"
+#include "pretty-print.h"
/* A pointer to one of the hooks containers. */
static struct cfg_hooks *cfg_hooks;
@@ -308,6 +309,10 @@ dump_bb_for_graph (pretty_printer *pp, b
if (!cfg_hooks->dump_bb_for_graph)
internal_error ("%s does not support dump_bb_for_graph",
cfg_hooks->name);
+ if (bb->count)
+pp_printf (pp, "COUNT:" HOST_WIDEST_INT_PRINT_DEC, bb->count);
+ pp_printf (pp, " FREQ:%i |", bb->frequency);
+ pp_write_text_to_stream (pp);
cfg_hooks->dump_bb_for_graph (pp, bb);
}
Index: graph.c
===
--- graph.c (revision 198108)
+++ graph.c (working copy)
@@ -155,11 +155,12 @@ draw_cfg_node_succ_edges (pretty_printer
pp_printf (pp,
"\tfn_%d_basic_block_%d:s -> fn_%d_basic_block_%d:n "
-"[style=%s,color=%s,weight=%d,constraint=%s];\n",
+"[style=%s,color=%s,weight=%d,constraint=%s,
label=\"[%i%%]\"];\n",
funcdef_no, e->src->index,
funcdef_no, e->dest->index,
style, color, weight,
-(e->flags & (EDGE_FAKE | EDGE_DFS_BACK)) ? "false" : "true");
+(e->flags & (EDGE_FAKE | EDGE_DFS_BACK)) ? "false" : "true",
+e->probability * 100 / REG_BR_PROB_BASE);
}
pp_flush (pp);
}
Index: Makefile.in
===
--- Makefile.in (revision 198108)
+++ Makefile.in (working copy)
@@ -3123,7 +3123,7 @@ cfg.o : cfg.c $(CONFIG_H) $(SYSTEM_H) co
$(GGC_H) $(OBSTACK_H) alloc-pool.h $(HASH_TABLE_H) $(CFGLOOP_H) $(TREE_H) \
$(BASIC_BLOCK_H)
cfghooks.o: cfghooks.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(RTL_H) \
- $(TREE_H) $(BASIC_BLOCK_H) $(TREE_FLOW_H) $(TIMEVAR_H) toplev.h
$(DIAGNOSTIC_CORE_H) $(CFGLOOP_H)
+ $(TREE_H) $(BASIC_BLOCK_H) $(TREE_FLOW_H) $(TIMEVAR_H) toplev.h
$(DIAGNOSTIC_CORE_H) $(CFGLOOP_H) $(PRETTY_PRINT_H)
cfgexpand.o : cfgexpand.c $(TREE_FLOW_H) $(CONFIG_H) $(SYSTEM_H) \
$(RTL_H) $(TREE_H) $(TM_P_H) $(EXPR_H) $(FUNCTION_H) $(TM_H) \
coretypes.h $(EXCEPT_H) langhooks.h $(TREE_PASS_H) $(RTL_H) \
On Sat, Apr 20, 2013 at 4:02 PM, Steven Bosscher wrote:
> On Sun, Apr 21, 2013 at 12:47 AM, Xinliang David Li wrote:
>> Index: graph.c
>> ===
>> --- graph.c (revision 198108)
>> +++ graph.c (working copy)
>> @@ -110,6 +110,9 @@ draw_cfg_node (pretty_printer *pp, int f
>>else
>> {
>>pp_character (pp, '{');
>> + if (bb->count)
>> + pp_printf (pp, "COUNT:" HOST_WIDEST_INT_PRINT_DEC, bb->count);
>> + pp_printf (pp, " FREQ:%i |", bb->frequency);
>>pp_write_text_to_stream (pp);
>>dump_bb_for_graph (pp, bb);
>>pp_character (pp, '}');
>
> This doesn't belong here, please put it in cfghooks.c:dump_bb_for_graph.
>
> Otherwise, looks good me.
>
> Ciao!
> Steven
