usx95 wrote: > We should not have flow from r to p but we do. Why? Because r is a > subexpression of the ternary and the fact generation will only look at the > subexpressions of the ternary without consulting the CFG whether we actually > have an edge between those AST nodes in the control flow.
Thanks for clarification. Sounds good to me then. > I'm wondering if the cleanest solution would be something like: > > * Get the Cfg node corresponding to evaluating the condition. The successors > of this node are the branches. > * Check if those branches are sink nodes, as in they never reach the end of > the function call. You could use `isInevitablySinking` and do the flow only > if this API returns false. > > I think this might handle all of the complex cases more robustly. > > That being said, `isInevitablySinking` looks like a relatively expensive API > to call and calling it on multiple disparate CFGNodes seem to do some > redundant work as the subtasks for the commonly reachable nodes are not > cached. So I am not 100% sure if we actually need this precision at the cost > of too much computation. > The best solution might be to make `isInevitablySinking` a bit cheaper and > use it for the flow but that might be a bit more work. I like this. While we are at it, I think it might be worth pursuing. It looks like that it would be relatively simpler to introduce a cache for the DFS in `isInevitablySinking`, e.g. a 2-bit `SinkingCache` in `CFGBlock` to memoize the reachability results (uncomputed, sinking, or not sinking). If there is any successor which is not-sinking, current block is not sinking. https://github.com/llvm/llvm-project/pull/190345 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
