<div dir="ltr">Hmm, I think I run clang-format only on the part that I changed (moving the Constant folding part of getNode into the separate static function) , because what I did was:<div><br></div><div>- Writing the FoldBinaryArithmetic function and modify the SelectionDAG file for the patch.</div>
<div>- Run clang-format on top of all the SelectionDAG.cpp file</div><div>- Realizing this wasn't a good idea (:P)</div><div>- Starting back from a clean SelectionDAG.cpp file and integrating only the changes ( I simply copied the clang-formatted changes I made into the clean SelectionDAG.cpp).</div>
<div><br></div><div>Is it better to have two patches, one with the not clang-formatted FoldBinaryArithmetic and one with the clang-formatted version you think?</div><div><br></div><div>Marcello</div></div><div class="gmail_extra">
<br><br><div class="gmail_quote">2014-05-29 23:07 GMT-07:00 Owen Anderson <span dir="ltr"><<a href="mailto:resistor@mac.com" target="_blank">resistor@mac.com</a>></span>:<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">Hi Marcello,<div><br></div><div>I haven’t looked at the code changes, but it appears that you accidentally ran clang-format over all of SelectionDAG.cpp as part of flag_nodes.patch. Please separate your intended diff out from that.</div>
<div><br></div><div>—Owen</div><div><br><div><div><div class="h5"><div>On May 29, 2014, at 7:44 PM, Marcello Maggioni <<a href="mailto:hayarms@gmail.com" target="_blank">hayarms@gmail.com</a>> wrote:</div><br></div>
</div><blockquote type="cite"><div><div class="h5"><div dir="ltr"><span style="font-family:arial,sans-serif;font-size:13px">Hello,</span><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">
here is a proposed patch to pass down the BinaryOperator's "NoUnsignedWrap/NoSignedWrap and IsExact" flags down to the SDNodes so that backends can use them for additional optimizations.</div>
<div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">The core of the patch is using the SubclassData field of SDNode for BinarySDNodes to store the flags information.</div>
<div style="font-family:arial,sans-serif;font-size:13px">SelectionDAGBuilder will then set the correct flag information the moment the Binary instruction is visited during construction.</div><div style="font-family:arial,sans-serif;font-size:13px">
<br></div><div style="font-family:arial,sans-serif;font-size:13px">In doing this I also refactored a little bit the getNode() method accepting two operands, extracting the main constant folding logic into a separate static function.</div>
<div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">The patch implementing this is the file "flag_nodes.patch" attached.</div><div style="font-family:arial,sans-serif;font-size:13px">
<br></div><div style="font-family:arial,sans-serif;font-size:13px">In addition I implemented an optimization where an unneeded "test instruction" that was emitted from the X86 backend is not emitted anymore. This optimization uses this new feature.</div>
<div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">This optimization is contained in the "x86_opt_wrap.patch" attached.</div><div style="font-family:arial,sans-serif;font-size:13px">
<br></div><div style="font-family:arial,sans-serif;font-size:13px">I also added a test case that exercises the optimization in the "factorial_test.patch"</div><div style="font-family:arial,sans-serif;font-size:13px">
<br></div><div style="font-family:arial,sans-serif;font-size:13px">The patch makes this snippet of code:</div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">
LBB0_1: ## %while.body<br></div><div style="font-family:arial,sans-serif;font-size:13px"><div> ## =>This Inner Loop Header: Depth=1</div><div>
imulq %rdi, %rax</div><div> decq %rdi</div><div> testq %rdi, %rdi</div><div> jg LBB0_1</div></div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">
<br></div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">become this:</div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">
<div><br></div><div><br></div><div>LBB0_1: ## %while.body<br></div><div><div> ## =>This Inner Loop Header: Depth=1</div><div> imulq %rdi, %rax</div>
<div> decq %rdi</div><div> jg LBB0_1</div></div></div><div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">if the NSW flag is set on the SDNode that represents the DEC instruction (presumably an ADD or a SUB node in this case).</div>
<div style="font-family:arial,sans-serif;font-size:13px"><br></div><div style="font-family:arial,sans-serif;font-size:13px">If somebody could review this it would be awesome :)</div><div style="font-family:arial,sans-serif;font-size:13px">
<br></div><div style="font-family:arial,sans-serif;font-size:13px">Cheers,</div><div style="font-family:arial,sans-serif;font-size:13px">Marcello</div></div>
</div></div><span><factorial_test.patch></span><span><flag_nodes.patch></span><span><x86_opt_wrap.patch></span>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@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></blockquote></div><br></div></div></blockquote></div><br></div>