[llvm] r297026 - [DAGCombiner] simplify div/rem-by-0

[Friedman, Eli via llvm-commits]

On 3/6/2017 8:36 AM, Sanjay Patel via llvm-commits wrote:
> Author: spatel
> Date: Mon Mar  6 10:36:42 2017
> New Revision: 297026
>
> URL: http://llvm.org/viewvc/llvm-project?rev=297026&view=rev
> Log:
> [DAGCombiner] simplify div/rem-by-0
>
> Refactoring of duplicated code and more fixes to follow.
>
> This is motivated by the post-commit comments for r296699:
> http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20170306/435182.html
>
> Ie, we can crash if we're missing obvious simplifications like this that
> exist in the IR simplifier or if these occur later than expected.
>
> The x86 change for non-splat division shows a potential opportunity to improve
> vector codegen: we assumed that since only one lane had meaningful results, we
> should do the math in scalar. But that means moving back and forth from vector
> registers.

I don't think this fully solves the problem you're trying to solve.

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.
2. You're not performing the checks in the right order.

(You're also not checking for signed overflow, but I think it works by 
accident because of the way APInt::sdiv is implemented.)

Anyway, I think the right fix is to make 
SelectionDAG::FoldConstantArithmetic (the overload which takes two 
ConstantSDNode*) return undef for divide-by-zero rather than failing to 
fold.

-Eli

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project