Hi all,<div><br></div><div>Just FYI, I&#39;ve posted an updated version of the patch here:</div><div><a href="https://gist.github.com/db03ab15ef8b76246b84#file_checked_add_prototype.ver3.patch">https://gist.github.com/db03ab15ef8b76246b84#file_checked_add_prototype.ver3.patch</a></div>
<div><br></div><div>It pretty much implements what John suggested in a previous email (quoted below).</div><div><br></div><div>This version still suffers from a couple of problem mentioned before:</div><div>1. It&#39;s emitting a jmpConU instead of a jmpCon during instruction selection. Is there a way to force it use jmpCon here?</div>
<div>2. I had to use a fixed register for the sum projection of CheckedAddI, otherwise I couldn&#39;t find a way to specify this projection should use the same register as &quot;dst&quot;.</div><div><br></div><div>- Kris<br>
<br><div class="gmail_quote">On Wed, Jun 20, 2012 at 3:12 AM, John Rose <span dir="ltr">&lt;<a href="mailto:john.r.rose@oracle.com" target="_blank">john.r.rose@oracle.com</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word"><div><div class="im"><div>On Jun 19, 2012, at 7:07 AM, Krystal Mok wrote:</div><br><blockquote type="cite"><span style="border-collapse:separate;font-family:Helvetica;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:-webkit-auto;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;font-size:medium"><div>
In the DivMod example, Div and Mod nodes with the same inputs start out floating separately, and get fused into a single DivMod late in Optimize(). The DivMod node type is a MultiNode with 2 projections, one for the div and the other for the mod. There&#39;s special treatment of DivMod in Matcher, and custom logic to match the projections.</div>
<div><br></div><div>If I add a new node type AddIOverflow following the same model, I might get it like this:</div><div><a href="https://gist.github.com/bdd13666e4796b09f36e" target="_blank">https://gist.github.com/bdd13666e4796b09f36e</a></div>
<div>This node type would have 2 projections just like DivMod, one for the add, and the other for the overflow condition.</div></span></blockquote><div><br></div></div><div>(rename INT_CC &rArr; INT_CC_PAIR for clarity)</div><div class="im">
<br><blockquote type="cite">Then there are two questions:<span style="border-collapse:separate;font-family:Helvetica;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:-webkit-auto;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;font-size:medium"><div>
<br></div><div>1. I&#39;d need another new node type,&nbsp;presumably&nbsp;called &quot;CheckAddIOverflow&quot; here, that derives from CmpINode and acts as if it produces condition codes. AddI(a, b) and CheckAddIOverflow(a, b) float separately, just like the DivMod example. They get fused up late in Optimize() into the AddIOverflow shown above. Does this sound right?</div>
</span></blockquote><div><br></div></div><div>Yes, that sounds right. &nbsp;The names don&#39;t feel right, yet. &nbsp;What&#39;s the best term for a version of Add which is not subject to overflow? &nbsp;That&#39;s what the user will be trying to build. &nbsp;Maybe a word like &quot;Checked&quot; or &quot;Safe&quot; is best, rather than focusing on overflow.</div>
<div><br></div><div>Maybe: &nbsp;AddI(a, b) plus&nbsp;CmpAddI(a, b) &rArr; CheckedAddI(a, b) plus two projections yielding the first two values.</div><div>You might be right about adding two new BoolNode states. &nbsp;That way you can think in terms of:</div>
<div>&nbsp; if ((c = a&nbsp;+ b) != 0) &hellip;</div><div>turning into</div><div>&nbsp; c=AddI(a,b); cc=CmpAddI(a,b); if(Bool::nz(cc)) &hellip;</div><div><br></div><div>And overflow turns into the natural special case supported by the HW.</div><div class="im">
<div><br></div><blockquote type="cite">2. With AddIOverflow going into the Matcher, how should I write the match rule in the ad file, to match:&nbsp;If(Bool(Proj(AddIOverflow(a, b)).overflow, ne), labl) ?<span style="border-collapse:separate;font-family:Helvetica;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:-webkit-auto;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;font-size:medium"><div>
<br></div><div>Would it look like: match(If cop (AddIOverflow dest src)) ?</div></span></blockquote><div><br></div></div><div>(You have probably already noticed it; the matcher.cpp comment &quot;Convert (If (Bool (CmpX A B))) into (If (Bool) (CmpX A B))&quot; is highly relevant to matching If nodes. &nbsp;See also uses of BinaryNode.)</div>
<div><br></div><div>Following the DivMod pattern, the matcher (in its current state) will have to match the CheckedAddI as a stand-alone node. &nbsp;Subtrees of matches can only have one use; this is a key matcher invariant. &nbsp;But every DivMod or CheckedAddI will have two uses: &nbsp;Its two projections.</div>
<div><br></div><div>So you&#39;ll have:</div><div><br></div><div>&nbsp; &nbsp;match(CheckedAddI);</div><div><br></div><div>and the pre-existing rule:</div><div><br></div><div>&nbsp; &nbsp;If(cop cmp)</div><div><br></div><div>The cop will be Bool::ov (or Bool::nz in the example above) and the cmp will be CheckedAddI.cmp.</div>
<div><br></div><div>You might think about extending the matcher machine to allow rules which specify the disposition of both projections:</div><div><br></div><div>&nbsp; match( (If cop (Proj#1 (CheckedAddI a b))); (Set c (Proj#0 (CheckedAddI a b))));</div>
<div><br></div><div>This would be a Big Project. &nbsp;Long term, I believe it is worth thinking about ways to model instructions that do more than one thing at a time.</div><div class="im"><br><blockquote type="cite">If the pair of overflow/non-overflow conditions are in BoolTest and in cmpOp, perhaps I wouldn&#39;t need to match the If node, and could just let jmpCon rule handle it as-is.<span style="border-collapse:separate;font-family:Helvetica;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:-webkit-auto;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;font-size:medium"><div>
That way I just have to match&nbsp;AddIOverflow itself in the ad file, like DivMod.</div></span></blockquote></div></div><span class="HOEnZb"><font color="#888888"><br><div>&mdash; John</div></font></span></div></blockquote></div><br>
</div>