[PATCH] D47576: [InstCombine] Fix div handling

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 1 07:49:00 PDT 2018


spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

LGTM - see inline comments.



================
Comment at: lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:586
     --BBI;
-    // If we found a call to a function, we can't assume it will return, so
+    // If we found an instruction that we can't assume it will return, so
     // information from below it cannot be propagated above it.
----------------
Typo:
"If we found an instruction that we can't assume will return..."


================
Comment at: test/Transforms/InstCombine/sdiv-guard.ll:1
+; RUN: opt < %s -instcombine -inline -S | FileCheck %s
+
----------------
skatkov wrote:
> spatel wrote:
> > This test already passes without the patch. Please create a test that shows the diff and use utils/update_test_checks.py to generate checks for it.
> > 
> > Also, we shouldn't need -inline to show a failure in instcombine.
> Test actually shows the diff. Without this patch it will transform guard condition to %X != 0 which is incorrect because if X is not 0 and flag is false gaurd should be triggered.
Ah, the assertions were just not strong enough then. I committed the baseline test here:
rL333756

Please remove the 'FIXME' and update/rebase the patch.


https://reviews.llvm.org/D47576





More information about the llvm-commits mailing list