<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Feb 9, 2017 at 8:14 AM Daniel Berlin via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">dberlin created this revision.<br class="gmail_msg">
Herald added a reviewer: tstellarAMD.<br class="gmail_msg">
Herald added subscribers: nhaehnle, arsenm.<br class="gmail_msg">
<br class="gmail_msg">
Convert all obvious node_begin/node_end and child_begin/child_end<br class="gmail_msg">
pairs to range based for.<br class="gmail_msg">
<br class="gmail_msg">
Sending for review in case someone has a good idea how to make<br class="gmail_msg">
graph_children able to be inferred. It looks like it would require<br class="gmail_msg">
changing GraphTraits to be two argument or something. I presume<br class="gmail_msg">
inference does not happen because it would have to check every<br class="gmail_msg">
GraphTraits in the world to see if the noderef types matched.<br class="gmail_msg"></blockquote><div><br></div><div>Yep, that. One could use an extra trait for the node type to map it back to the graph trait to facilitate this - but that'd also mean each node type would have to be unique to the graph type (which is probably true - but one could imagine different graphs over the same nodes? Maybe... maybe not)<br><br>Did you consider changing the trait so they exposed range accessors directly, instead of adding utility functions?<br>"graph_children(graph)" is a bit of an awkward construct. Alternatively maybe drop the graph_ prefix - ADL/overload resolution/etc will make it correct anyway, so just "children(graph)", etc?<br><br>(re: your other comment, I'm not sure I quite followed:<br><br>"<span style="color:rgb(33,33,33);font-size:13px">Note that this use of const auto & is required to make clang build, as the iterator type it for CFG blocks ends up returning blocks directly otherwise here." - not sure I quite follow. Best guess/reading is that the CFG's iterators' value type is heavyweight, but most other GraphTraits types have lightweight nodes that are cheap to copy? (like pointers, etc)<br></span></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="gmail_msg">
Note: This change was 3-staged with clang as well, which uses<br class="gmail_msg">
Dominators/etc from LLVM.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<a href="https://reviews.llvm.org/D29767" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D29767</a><br class="gmail_msg">
<br class="gmail_msg">
Files:<br class="gmail_msg">
  include/llvm/ADT/GraphTraits.h<br class="gmail_msg">
  include/llvm/Support/GenericDomTree.h<br class="gmail_msg">
  include/llvm/Support/GenericDomTreeConstruction.h<br class="gmail_msg">
  lib/Analysis/IteratedDominanceFrontier.cpp<br class="gmail_msg">
  lib/Target/AMDGPU/AMDILCFGStructurizer.cpp<br class="gmail_msg">
  lib/Target/Hexagon/HexagonBitSimplify.cpp<br class="gmail_msg">
  lib/Target/Hexagon/HexagonCommonGEP.cpp<br class="gmail_msg">
  lib/Target/Hexagon/HexagonGenExtract.cpp<br class="gmail_msg">
  lib/Target/Hexagon/HexagonGenInsert.cpp<br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div></div>