<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Apr 23, 2012, at 11:55 AM, Sirish Pande <<a href="mailto:spande@codeaurora.org">spande@codeaurora.org</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
<div bgcolor="#FFFFFF" text="#000000">
On 4/23/2012 1:29 PM, Chandler Carruth wrote:
<blockquote cite="mid:CAGCO0KiD8Y8Pq0VHwGhWBzJdHR=p=8xpj91X_bT4qffTLtCF0Q@mail.gmail.com" type="cite">
<div class="gmail_extra">
<div class="gmail_quote">On Mon, Apr 23, 2012 at 11:09 AM,
Sirish Pande <span dir="ltr"><<a moz-do-not-send="true" href="mailto:spande@codeaurora.org" target="_blank">spande@codeaurora.org</a>></span>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"> There were only two
reviews:<br>
<br>
1. Eric C asked for test cases, and that test cases be
associated with the patch.<br>
2. Tom Stellard had requested to split the vliw packetizer
patch.<br>
<br>
Both have been done.<br>
</div>
</blockquote>
<div><br>
</div>
<div>But the review isn't finished yet. You need to wait for
the reviewers to actually sign off on the change before
submitting. The two comments you got were what people needed
changed based on their first look, that doesn't mean the
code is ready to be submitted yet.</div>
<div><br>
</div>
<div>This patch is particularly problematic because it is not
just changing the Hexagon, it's changing a core part of the
codegen layer. Those changes need proper review before being
committed, and that hasn't happened yet.</div>
<div><br>
</div>
<div>I've reverted this patch until the review finishes
because the patches requiring it had to be reverted anyways.</div>
<div><br>
</div>
<div>I reverted the hexagon patches because your own
regression tests failed for all of the patches in the
series.</div>
<div><br>
</div>
<div>This gets the tree back into both a reviewed and
test-passing state. Let's keep it that way. =]</div>
</div>
</div>
</blockquote>
<br>
Fair enough. I will wait for the reviewers for sign off. <br></div></blockquote></div><br><div><div>This target-independent code looks ok to commit. (I haven't looked at your target-specific patches yet.)</div><div><br></div><div>As usual, under the following conditions</div><div>- no build warnings</div><div>- try to avoid significant increase in build times</div><div>- make check-all passes when configured for *all* targets</div><div> (this should apply to everyone committing target sensitive code, not just you!)</div><div>- Address buildbot failures that occured last time</div><div><br></div><div>Chandler mentioned that some bots showed failures, so please look into those and don't commit until you think you've fixed them.</div><div><br></div><div>A couple comments...</div><div><br></div><div>+ VLIWScheduler->exitRegion();</div><div><br></div><div>I'm not sure why you would do this *before* packetizing. It's ok if it works, just a little misleading. You should probably add a call to VLIWScheduler->finishBlock() somewhere.</div><div><br></div><div>+ // Generate MI -> SU map.</div><div>+ //std::map <MachineInstr*, SUnit*> MIToSUnit;</div><div>+ MIToSUnit.clear();</div><div>+ for (unsigned i = 0, e = VLIWScheduler->SUnits.size(); i != e; ++i) {</div><div>+ SUnit *SU = &VLIWScheduler->SUnits[i];</div><div>+ MIToSUnit[SU->getInstr()] = SU;</div><div>+ }</div><div><br></div><div>I added ScheduleDAGInstrs::MISUnitMap so you wouldn't need your own. I haven't tested it with a PostRA scheduler, but would like to know if it doesn't work.</div><div><br></div><div>Thanks!</div><div><br></div><div>-Andy</div></div><div><br></div></body></html>