> On Aug 13, 2025, at 05:24, Richard Biener <richard.guent...@gmail.com> wrote: > > On Wed, Jul 30, 2025 at 4:42 PM Qing Zhao <qing.z...@oracle.com> wrote: >> >> >> + /* Get the edge from the prev_cond_bb to cur_bb, to determine >> whether >> + the stmt is on the taken path of the conditional statement. */ >> + edge e = find_edge (prev_cond_bb, cur_bb); >> + bool is_branch_taken = BRANCH_EDGE (prev_cond_bb) == e; > > Using BRANCH_EDGE does only work in cfgrtl, you should instead do > > is_branch_taken = e->flags & EDGE_TRUE_VALUE > >> + path->add_event (gimple_location (cond_stmt), cfun->decl, 1, >> + "when the condition is evaluated to %s", >> + is_branch_taken ? "true" : "false"); >> + } >> + cur_bb = prev_cond_bb; >> + prev_cond_bb = single_pred_p (cur_bb) >> + ? single_pred (cur_bb) : NULL; > > Why do we only continue for single preds here? We should iterate to the > immediate dominator.
It’s because that the condition stmt at the end of the single pred and the edge from the single pred to current block is the information that we are interested in, for example the following CFG (the example in the comments of the file diagnostic-context-rich-location.cc). B2 / \ V \ B3 \ / \ \ V V \ B4 B5 V \ / B12 V B6 / \ V V B7 B11 / \ V V B8 B9 \ / V B10 (warning here) When issuing warning at B10, the interesting conditions we should report are: depth=1: the condition stmt at B6, the edge from B6->B7; depth=2: the condition stmt at B2, the edge from B2->B3; To get the above B6, we should get the single pred of B10 first, but since B10 doesn’t have a single pred, we then should backtrace to its immediate dominator, B7, and then try to get the single pred of B7, which is B6. At this point, we got the single pred, and the condition stmt at this single pred and the edge from this single pred to B7 is the one that we are interested. Then for depth=2, we start from B6 as the cur_bb, repeat the above step. So, in this heuristic, the key is to locate the single pred block of the current block, the interested COND and TRUE/FALSE information is embedded in the single pred block and the edge from single pred block to current block. If the current block doesn’t have a single pred block, then we should locate its immediate dominator, and treat this immediate dominator as the current block. Hope this is clear. > > So the overall CFG walk should be like > > do > { > cond_bb = get_immediate_dominator (CDI_DOMINATORS, cur_bb); The interesting cond_bb is NOT the immediate dominator of cur_bb, it should be the single predecessor of cur_bb, If cur_bb doesn’t have a single predecessor, then we should backtrace to its immediate dominator to get its single predecessor. > if (single_pred_p (curr_bb)) > { > auto gsi = gsi_last_bb (cond_bb); > if (!gsi_end_p (gsi) > && stmt_ends_bb_p (gsi_stmt (gsi))) > { > ... handle control stmt at the end on cond_bb ... Yes, but the cond_bb should be the single predecessor of cur_bb (or the single predecessor of the immediate dominator of cur_bb). Qing > } > } > curr_bb = cond_bb; > } > while (depth < flag_diagnostics_show_context > && curr_bb != ENTRY_BLOCK_PTR_FOR_FN (cfun)); > > stmt_ends_bb_p is is_ctrl_stmt () || is_ctrl_altering_stmt where the > former includes GIMPLE_COND (and switch and more) and the > latter includes for example a call that internally throws. Whether > we terminate the walk upon unhandled control stmts would be > up to debate, I'd skip for now, as you do and as you also skip > GIMPLE_COND without location. > >> + } >> + while (prev_cond_bb && depth < flag_diagnostics_show_context); >> + if (should_terminate) >> + break; >> + } >> + while (prev_cond_bb != ENTRY_BLOCK_PTR_FOR_FN (cfun) >> + && depth < flag_diagnostics_show_context); >> + >> + >> + /* Add an end of path warning event in the end of the path. */ >> + if (path->num_events () > 0) >> + path->add_event (m_location, cfun->decl, 1, >> + "warning happens here"); >> + return path; >> +} >> diff --git a/gcc/diagnostic-context-rich-location.h >> b/gcc/diagnostic-context-rich-location.