<div dir="ltr"><div>Thanks, Eli!<br><br>No objections that this doesn't solve the motivating problem or that the ordering is bogus. I was planning to address that next, but your suggestion to snuff this out in FoldConstantArithmetic does seem better. Let me check the related handling in IR and see if we can do better across the board.<br><br></div>About vectors: I thought we are not allowed to assume that undef in one lane allows simplification of the whole vector. Ie, undef is not "infectious" across lanes. I can't remember where I last saw that though... Not sure if this would be different in IR vs. DAG?<br><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 6, 2017 at 10:32 AM, Friedman, Eli <span dir="ltr"><<a href="mailto:efriedma@codeaurora.org" target="_blank">efriedma@codeaurora.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 3/6/2017 8:36 AM, Sanjay Patel via llvm-commits wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: spatel<br>
Date: Mon Mar  6 10:36:42 2017<br>
New Revision: 297026<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=297026&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=297026&view=rev</a><br>
Log:<br>
[DAGCombiner] simplify div/rem-by-0<br>
<br>
Refactoring of duplicated code and more fixes to follow.<br>
<br>
This is motivated by the post-commit comments for r296699:<br>
<a href="http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20170306/435182.html" rel="noreferrer" target="_blank">http://lists.llvm.org/pipermai<wbr>l/llvm-commits/Week-of-Mon-<wbr>20170306/435182.html</a><br>
<br>
Ie, we can crash if we're missing obvious simplifications like this that<br>
exist in the IR simplifier or if these occur later than expected.<br>
<br>
The x86 change for non-splat division shows a potential opportunity to improve<br>
vector codegen: we assumed that since only one lane had meaningful results, we<br>
should do the math in scalar. But that means moving back and forth from vector<br>
registers.<br>
</blockquote>
<br></span>
I don't think this fully solves the problem you're trying to solve.<br>
<br>
1. For a vector divide by constant, I think you need to check whether any lane is null, not just whether it's null as a whole.<br>
2. You're not performing the checks in the right order.<br>
<br>
(You're also not checking for signed overflow, but I think it works by accident because of the way APInt::sdiv is implemented.)<br>
<br>
Anyway, I think the right fix is to make SelectionDAG::FoldConstantArit<wbr>hmetic (the overload which takes two ConstantSDNode*) return undef for divide-by-zero rather than failing to fold.<span class="HOEnZb"><font color="#888888"><br>
<br>
-Eli<br>
<br>
-- <br>
Employee of Qualcomm Innovation Center, Inc.<br>
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project<br>
<br>
</font></span></blockquote></div><br></div>