<div dir="ltr"><span style="font-family:arial,sans-serif;font-size:13px">Hi, thanks Daniil for your review!</span><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">
1) Yeah, I also kind of like what you proposed more ... I used the approach that is used currently in BinaryOperator for that. I'm not sure if it is better to change both to this format or having them to diverge (keeping BinaryOperator like it was and changing this one to the syntax you proposed).</div>
<div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">2) Thanks for spotting those stylistic inconsistencies! I'm working on a new setup and I think I still have to tune some of my editor default settings ...</div>
<div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">3) What you mean exactly with this one?</div><div style="font-family:arial,sans-serif;font-size:13px">
<br></div><div style="font-family:arial,sans-serif;font-size:13px">PS = Here are some updated patches with concerns number 1 and 2 addressed.</div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">
Thanks,</div><div style="font-family:arial,sans-serif;font-size:13px">Marcello</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">2014-06-02 17:00 GMT-07:00 Marcello Maggioni <span dir="ltr"><<a href="mailto:hayarms@gmail.com" target="_blank">hayarms@gmail.com</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi, thanks Daniil for your review!<div><br></div><div>1) Yeah, I also kind of like what you proposed more ... I used the approach that is used currently in BinaryOperator for that. I'm not sure if it is better to change both to this format or having them to diverge (keeping BinaryOperator like it was and changing this one to the syntax you proposed).</div>
<div><br></div><div>2) Thanks for spotting those stylistic inconsistencies! I'm working on a new setup and I think I still have to tune some of my editor default settings ...</div><div><br></div><div>3) What you mean exactly with this one?</div>
<div><br></div><div>PS = Here are some updated patches with concerns number 1 and 2 addressed.</div><div><br></div><div>Thanks,</div>
<div>Marcello</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">2014-06-02 2:36 GMT-07:00 Данил Трошков <span dir="ltr"><<a href="mailto:troshkovdanil@mail.ru" target="_blank">troshkovdanil@mail.ru</a>></span>:<div>
<div class="h5"><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>1)<div><br> (SubclassData & ~NUW) | (b * NUW);<br></div>Maybe better to write somehow<div><br>(SubclassData & ~NUW) | (b ? NUW : 0);<br></div>IMHO it is more readable<br><br>2) const BinarySDNode *BinNode = static_cast<const BinarySDNode*>(N);<br>
formatting: \t...<br>+ BinNode->hasNoUnsignedWrap(),<br>+ BinNode->hasNoSignedWrap(),<br>+ BinNode->isExact());<br>the same...<br>+ bool nsw, bool exact) {<br>+ bool nsw, bool exact) {<br>
+ Op1, Op2, HasNUW, HasNSW, IsExact);<br>+ if (BinNode->hasNoSignedWrap())<br>+ break;<br><br>3) Maybe better to remove bundle of cases on low level?<br>In order to reduce duplication of code...<br><br><br><br>
Sat, 31 May 2014 19:30:24 -0700 от Marcello Maggioni <<a href="mailto:hayarms@gmail.com" target="_blank">hayarms@gmail.com</a>>:<br>
<blockquote style="margin:10px;padding:0px 0px 0px 10px;border-left-color:rgb(8,87,166);border-left-width:1px;border-left-style:solid">
<div>
<div>
<div><div><div>
<div><div dir="ltr">Here an updated patch.<div><br></div><div>The changes are:</div><div><br></div><div>- I stripped away the new getBinaryNode function and integrated the functionality in the getNode() function with two operands.</div>
<div>- I removed the refactoring for the binary folding logic. Removing getBinaryNode() makes putting that logic in an external function not needed. (this also fixes the clang-format problem Owen pointed out).</div><div>
- Cleaned up some stuff and fixed some style problems.</div>
<div>- Integrated the changes Pete pointed out:</div><div> - Added the flags to the NodeID construction</div><div> - Created a test case for the CSE (not sure it is the best test though ... I'm not an expert testing this kind of stuff)</div>
<div> - Merged the test for the performance optimization in the optimization patch.</div><div> - Integrated the code cleanups that were specified.</div><div> - Reduced the test case for the x86 opt.</div><div><br></div>
<div>Now as a result there are only two patches:</div><div> - flag_nodes.patch</div><div> - x86_opt_wrap.patch</div><div><br></div><div>I didn't changed BinarySDNode to have classof(), because I think is probably better doing it separately , but also because it is kind of more complex than it seems , because it is not clear exactly what are the opcodes that become BinarySDNodes (a part from the obvious binary ADD, SUB ... etc operations).</div>
<div>It is probably necessary to take more time to collect an accurate list of all the opcodes that always end up being lowered as BinarySDNodes and add the classof() then.</div><div><br></div><div>Marcello</div><div><br>
</div><div><br></div></div>
</div>
</div></div><div><div>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="https://e.mail.ru/compose?To=llvm%2dcommits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
</div>
</div></div>
</div>
</div>
</blockquote>
<br></div>
</blockquote></div></div></div><br></div>
</blockquote></div><br></div>