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

[Sanjay Patel via llvm-commits]

Thanks, Eli!

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.

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?


On Mon, Mar 6, 2017 at 10:32 AM, Friedman, Eli <efriedma at codeaurora.org>
wrote:

> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170306/f9062e3c/attachment.html>