[llvm-bugs] [Bug 43376] New: InstCombine: llvm::getFlippedStrictnessPredicateAndConstant(CmpInst::Predicate, llvm::Constant *): Assertion `ConstantIsOk(cast<ConstantInt>(C))' failed.

Fri Sep 20 04:11:38 PDT 2019

https://bugs.llvm.org/show_bug.cgi?id=43376

            Bug ID: 43376
           Summary: InstCombine:
                    llvm::getFlippedStrictnessPredicateAndConstant(CmpInst
                    ::Predicate, llvm::Constant *): Assertion
                    `ConstantIsOk(cast<ConstantInt>(C))' failed.
           Product: libraries
           Version: trunk
          Hardware: PC
                OS: Windows NT
            Status: NEW
          Severity: enhancement
          Priority: P
         Component: Scalar Optimizations
          Assignee: unassignedbugs at nondot.org
          Reporter: bjorn.a.pettersson at ericsson.com
                CC: llvm-bugs at lists.llvm.org

InstCombine hits an assertion given the following test case (bbi-32675.ll):


define i16 @d(i16* %d.a, i16* %d.b) {
entry:
  %t0 = load i16, i16* %d.a, align 1
  %tobool = icmp ne i16 %t0, 0
  br i1 %tobool, label %land.rhs, label %land.end

land.rhs:
  %t1 = load i16, i16* %d.b, align 1
  %cmp = icmp ult i16 %t1, 0
  br label %land.end

land.end:
  %t2 = phi i1 [ false, %entry ], [ %cmp, %land.rhs ]
  %land.ext = zext i1 %t2 to i16
  %mul = mul nsw i16 %land.ext, 3
  %neg = xor i16 %mul, -1
  ret i16 %neg
}


opt -instcombine -o - bbi-32675.ll -S
opt: ../lib/Transforms/InstCombine/InstCombineCompares.cpp:5143:
llvm::Optional<std::pair<CmpInst::Predicate, Constant *> >
llvm::getFlippedStrictnessPredicateAndConstant(CmpInst::Predicate,
llvm::Constant *): Assertion `ConstantIsOk(cast<ConstantInt>(C))' failed.
Stack dump:
0.      Program arguments: ../../llvm-upstream//llvm/build-all/bin/opt
-instcombine -o - bbi-32675.ll -S 
1.      Running pass 'Function Pass Manager' on module 'bbi-32675.ll'.
2.      Running pass 'Combine redundant instructions' on function '@d'
 #0 0x000000000240c9f4 PrintStackTraceSignalHandler(void*)
(../../llvm-upstream//llvm/build-all/bin/opt+0x240c9f4)
 #1 0x000000000240a71e llvm::sys::RunSignalHandlers()
(../../llvm-upstream//llvm/build-all/bin/opt+0x240a71e)
 #2 0x000000000240cdf8 SignalHandler(int)
(../../llvm-upstream//llvm/build-all/bin/opt+0x240cdf8)
 #3 0x0000003ba280f7e0 __restore_rt (/lib64/libpthread.so.0+0x3ba280f7e0)
 #4 0x0000003ba24324f5 raise (/lib64/libc.so.6+0x3ba24324f5)
 #5 0x0000003ba2433cd5 abort (/lib64/libc.so.6+0x3ba2433cd5)
 #6 0x0000003ba242b66e __assert_fail_base (/lib64/libc.so.6+0x3ba242b66e)
 #7 0x0000003ba242b730 __assert_perror_fail (/lib64/libc.so.6+0x3ba242b730)
 #8 0x0000000001f92be1
llvm::getFlippedStrictnessPredicateAndConstant(llvm::CmpInst::Predicate,
llvm::Constant*) (../../llvm-upstream//llvm/build-all/bin/opt+0x1f92be1)
 #9 0x0000000001fc858d
llvm::InstCombiner::foldSelectInstWithICmp(llvm::SelectInst&, llvm::ICmpInst*)
(../../llvm-upstream//llvm/build-all/bin/opt+0x1fc858d)
#10 0x0000000001fcac4c llvm::InstCombiner::visitSelectInst(llvm::SelectInst&)
(../../llvm-upstream//llvm/build-all/bin/opt+0x1fcac4c)
#11 0x0000000001f27f8f llvm::InstCombiner::run()
(../../llvm-upstream//llvm/build-all/bin/opt+0x1f27f8f)
#12 0x0000000001f2a2f1 combineInstructionsOverFunction(llvm::Function&,
llvm::InstCombineWorklist&, llvm::AAResults*, llvm::AssumptionCache&,
llvm::TargetLibraryInfo&, llvm::DominatorTree&,
llvm::OptimizationRemarkEmitter&, llvm::BlockFrequencyInfo*,
llvm::ProfileSummaryInfo*, bool, llvm::LoopInfo*)
(../../llvm-upstream//llvm/build-all/bin/opt+0x1f2a2f1)
#13 0x0000000001f2aaba
llvm::InstructionCombiningPass::runOnFunction(llvm::Function&)
(../../llvm-upstream//llvm/build-all/bin/opt+0x1f2aaba)
#14 0x0000000001d8ae13 llvm::FPPassManager::runOnFunction(llvm::Function&)
(../../llvm-upstream//llvm/build-all/bin/opt+0x1d8ae13)
#15 0x0000000001d8b123 llvm::FPPassManager::runOnModule(llvm::Module&)
(../../llvm-upstream//llvm/build-all/bin/opt+0x1d8b123)
#16 0x0000000001d8b78d llvm::legacy::PassManagerImpl::run(llvm::Module&)
(../../llvm-upstream//llvm/build-all/bin/opt+0x1d8b78d)
#17 0x00000000007d1617 main
(../../llvm-upstream//llvm/build-all/bin/opt+0x7d1617)
#18 0x0000003ba241ed20 __libc_start_main (/lib64/libc.so.6+0x3ba241ed20)
#19 0x00000000007b8d39 _start
(../../llvm-upstream//llvm/build-all/bin/opt+0x7b8d39)
Abort (core dumped)

This was seen with trunk at r371985.


Quick analysis:
Problem seem to be that the code in
llvm::getFlippedStrictnessPredicateAndConstant assumes that "For scalars,
SimplifyICmpInst should have already handled the edge cases for us, so we just
assert on them.". But looking at the worklist order in InstCombine the
instructions will be visited in the order:

  %t0 = load i16, i16* %d.a, align 1
  %tobool = icmp ne i16 %t0, 0
  br i1 %tobool, label %land.rhs, label %land.end
  %t2 = phi i1 [ false, %entry ], [ %cmp, %land.rhs ]
  %land.ext = zext i1 %t2 to i16
  %mul = mul nsw i16 %land.ext, 3
  %neg = xor i16 %mul, -1
  ret i16 %neg
  %t1 = load i16, i16* %d.b, align 1
  %cmp = icmp ult i16 %t1, 0
  br label %land.end

So the phi using %cmp (which will become a select) is visited before the icmp
that evaluates %cmp, and the assumption that SimplifyICmpInst should have
handled the edge case for %cmp already does not hold.

A simple workround could be to replace the assert like this:

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index e569ae866b5..13c0b4ac0a3 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -5132,12 +5132,14 @@
llvm::getFlippedStrictnessPredicateAndConstant(CmpInst::Predicate Pred,
     return WillIncrement ? !C->isMaxValue(IsSigned) :
!C->isMinValue(IsSigned);
   };

-  // For scalars, SimplifyICmpInst should have already handled
-  // the edge cases for us, so we just assert on them.
+  // For scalars, SimplifyICmpInst should have already handled the edge cases
+  // for us, although that depends on if we already have visited the ICmp
+  // instruction or not (we might have to wait for the next iteration).
   // For vectors, we must handle the edge cases.
   if (isa<ConstantInt>(C)) {
+    if (!ConstantIsOk(cast<ConstantInt>(C)))
+      return llvm::None;
     // A <= MAX -> TRUE ; A >= MIN -> TRUE
-    assert(ConstantIsOk(cast<ConstantInt>(C)));
   } else if (Type->isVectorTy()) {
     // TODO? If the edge cases for vectors were guaranteed to be handled as
they
     // are for scalar, we could remove the min/max checks. However, to do
that,

I'd assume that returning llvm::None will prevent any optimization right now,
but since InstCombine is iterative we could be successful in the next iteration
instead (after having simplified the icmp). At the cost of triggering yet
another iteration.

Maybe there is some more elegant way to solve this?

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20190920/12755da5/attachment-0001.html>
