[PATCH] D27216: [SCEVExpand] do not hoist divisions by zero (PR30935)

[Andrew Trick via llvm-commits]

> On Nov 30, 2016, at 1:16 AM, Sanjoy Das via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> sanjoy added a comment.
> 
> Hi,
> 
> I don't think this patch properly addresses PR30976 -- it only hides the symptom.  For instance, this new test case crashes SCEVExpander even with this fix: https://reviews.llvm.org/differential/diff/79708/ <https://reviews.llvm.org/differential/diff/79708/>
> 
> I think the fundamental problem is that there is no clear distinction in SCEVExpander between hoisting for optimization and hoisting for correctness (i.e. for the right domination properties to hold). We need to fix that before we can fix PR30976.

I didn’t look closely at this, but in general SCEVExpander doesn’t have the information to determine whether an expansion would be legal (let alone profitable). That all needs to be determined before ever calling SCEVExpander. If you need to limit code motion within an expression, then you need to break up the expression and call SCEVExpander separately on each subexpression.


-Andy
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161130/34c4dcee/attachment.html>