[llvm-bugs] [Bug 24873] New: InstCombine: Incorrect (icmp (ashr ) ) optimization

[via llvm-bugs]

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

            Bug ID: 24873
           Summary: InstCombine: Incorrect (icmp (ashr ) ) optimization
           Product: libraries
           Version: trunk
          Hardware: PC
                OS: All
            Status: NEW
          Severity: normal
          Priority: P
         Component: Scalar Optimizations
          Assignee: unassignedbugs at nondot.org
          Reporter: james.molloy at arm.com
                CC: llvm-bugs at lists.llvm.org
    Classification: Unclassified

I've confirmed that this bug has been latent since at least March this year,
but probably long before.

The attached file produces wrong code when run through opt -O1. The symptom of
this is "store i1 true, i1* @failed" indicating that the optimizer has folded
the testcase to a fallacy.

This is the interesting bit:

define i1 @f(i64 %arg) {
  %x = ashr i64 -4250131414448504832, %c
  %y = icmp eq i64 %x, -1
  ret i1 %y
}

If this is run through -instcombine in isolation it folds to the expected "i1
true", however in the attached testcase it does not, it folds to "i1 false".

This is because in the attached testcase the order that instcombine looks at
instructions is such that it looks at %y BEFORE it has folded %x to -1. I
cannot replicate this search order in a more reduced testcase.

In the testcase, the function InstructionCombiner::FoldICmpCstShrCst() is
executed. It folds the icmp into "icmp eq i64 63, 62".

The interesting bit is here:

  // Get the distance between the highest bit that's set.
  int Shift;
  // Both the constants are negative, take their positive to calculate log.
  if (IsAShr && AP1.isNegative())
    Shift = (~AP2).logBase2() - (~AP1).logBase2();
  else
    Shift = AP2.logBase2() - AP1.logBase2();

In this case, AP1 = -1, so ~AP1 = 0. lg2(0) is of course infinity but APInt
returns (int)~0 which is interpreted as -1. 61 - -1 = 62.

I'm unfortunately not quite sure enough about the maths going on here to work
out what we should do. Should we bail? can we compute a correct value somehow
(the correct Shift value being 63)

More importantly how on earth do we test things like this?! The random testcase
I generated pointed to a commit in reassociate that exposed the problem - that
was purely commuting the operands to an OR instruction. That changed the search
order and... bang :/

-- 
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/20150918/e431c43c/attachment.html>