<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Sep 29, 2016, at 1:15 PM, Matthias Braun <<a href="mailto:matze@braunis.de" class="">matze@braunis.de</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""><br class="Apple-interchange-newline">On Sep 29, 2016, at 12:54 PM, Andrew Trick via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class=""><br class=""><br class=""><blockquote type="cite" class="">On Sep 29, 2016, at 11:20 AM, Matthias Braun <<a href="mailto:matze@braunis.de" class="">matze@braunis.de</a>> wrote:<br class=""><br class=""><br class=""><blockquote type="cite" class="">On Sep 28, 2016, at 10:01 PM, Andrew Trick via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class=""><br class="">atrick added a comment.<br class=""><br class="">Marking a DAG node as "dead" for the purpose of scheduling should be an easy thing to do, relative to supporting instruction bundles. But adding the extra DAG edges is also a fine solution, just not quite as direct.<br class=""></blockquote><br class="">Marking a DAG as dead is still a more complex patch than this which is a strict improvement over the status quo and "contained" within the MacroFusionDAGMutation. So is this good to go?<br class=""><br class="">- Matthias<br class=""></blockquote><br class="">The review site seems down… I glanced at your attached patch. It looks like you’re traversing every edge in the DAG. Why don’t you just revisit the DAG leaves and immediate predecessors of the exitSU?<br class=""></blockquote><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">I wasn't aware any node is necessarily connected to the ExitSU (= ExitSU is the only node without any successors). I'll simplify the code with that assumption.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote><div><br class=""></div></div>Actually the code in findRootsAndBiasEdges seems to indicate that you cannot just walk the predecessors of ExitSU to find all roots... So maybe the current patch that checks all nodes in the graph is the only way to go.<div class=""><br class=""></div><div class="">- Matthias</div></body></html>