Hi Richard,
on 2019/7/11 上午3:30, Richard Sandiford wrote:
> "Kewen.Lin" <li...@linux.ibm.com> writes:
>> Hi all,
>> Is it a reasonable patch? If yes, is it ok for trunk?
>
> It looks really useful, but IMO we should either emit the hint for all
> end-of-block insns with a surprising fallthrough edge, or -- if we
> really do want to keep it specific to conditional jumps -- we should
> replace rhs "pc" in the jump insn with something like "bb <N>".
> Personally the former seems better to me (and should also be simpler).
>
Thanks a lot for the comments. It's a good idea not only to check conditional
jump insn. I've updated the patch accordingly. I was intended to change the
pc dumping by using the actual BB label similar to what we see for non
fallthrough edge. But as you mentioned it's not simple, need to pass some
information down, it looks like a hint is enough for this case. :)
>> + basic_block dest = e->dest;
>> + if (BB_HEAD (dest) != ninsn)
>> + fprintf (outf, ";; pc falls through to BB %d\n", dest->index);
>
> Very minor, but I think the output would be more readable if the comment
> were indented by the same amount as the register notes (like REG_DEAD above).
> In lisp tradition I guess the comment marker would then be ";" rather
> than ";;".
>
Good idea, also updated it accordingly, thanks! With the attached patch, the
dumping is as below:
12: r135:CC=cmp(r122:DI,0)
13: pc={(r135:CC!=0)?L52:pc}
REG_DEAD r135:CC
REG_BR_PROB 1041558836
; pc falls through to BB 10
31: L31:
17: NOTE_INSN_BASIC_BLOCK 3
Thanks,
Kewen
--------------
gcc/ChangeLog
2019-07-11 Kewen Lin <li...@gcc.gnu.org>
* gcc/cfgrtl.c (print_rtl_with_bb): Emit a hint if the fallthrough
target of current basic block isn't the placed right next.
diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c
index a1ca5992c41..5f2accf1f4f 100644
--- a/gcc/cfgrtl.c
+++ b/gcc/cfgrtl.c
@@ -2193,14 +2193,14 @@ print_rtl_with_bb (FILE *outf, const rtx_insn
*rtx_first, dump_flags_t flags)
if (df)
df_dump_start (outf);
- if (flags & TDF_BLOCKS)
+ FOR_EACH_BB_REVERSE_FN (bb, cfun)
{
- FOR_EACH_BB_REVERSE_FN (bb, cfun)
- {
- rtx_insn *x;
+ rtx_insn *x;
- start[INSN_UID (BB_HEAD (bb))] = bb;
- end[INSN_UID (BB_END (bb))] = bb;
+ start[INSN_UID (BB_HEAD (bb))] = bb;
+ end[INSN_UID (BB_END (bb))] = bb;
+ if (flags & TDF_BLOCKS)
+ {
for (x = BB_HEAD (bb); x != NULL_RTX; x = NEXT_INSN (x))
{
enum bb_state state = IN_MULTIPLE_BB;
@@ -2244,16 +2244,31 @@ print_rtl_with_bb (FILE *outf, const rtx_insn
*rtx_first, dump_flags_t flags)
if (flags & TDF_DETAILS)
df_dump_insn_bottom (tmp_rtx, outf);
- if (flags & TDF_BLOCKS)
+ bb = end[INSN_UID (tmp_rtx)];
+ if (bb != NULL)
{
- bb = end[INSN_UID (tmp_rtx)];
- if (bb != NULL)
+ if (flags & TDF_BLOCKS)
{
dump_bb_info (outf, bb, 0, dump_flags, false, true);
if (df && (flags & TDF_DETAILS))
df_dump_bottom (bb, outf);
putc ('\n', outf);
}
+ /* Emit a hint if the fallthrough target of current basic block
+ isn't the one placed right next. */
+ else if (EDGE_COUNT (bb->succs) > 0)
+ {
+ gcc_assert (BB_END (bb) == tmp_rtx);
+ const rtx_insn *ninsn = NEXT_INSN (tmp_rtx);
+ edge e = find_fallthru_edge (bb->succs);
+ if (e && ninsn)
+ {
+ basic_block dest = e->dest;
+ if (start[INSN_UID (ninsn)] != dest)
+ fprintf (outf, "%s ; pc falls through to BB %d\n",
+ print_rtx_head, dest->index);
+ }
+ }
}
}
