[PATCH] D54239: [JumpThreading] Fix exponential time algorithm computing known values.

Max Kazantsev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 7 22:30:54 PST 2018


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

LGTM.

It seems that the initial design of this algorithm wasn't intended to make 2 recursive calls from 1 place. The interesting bit is that we only can have an exponential explosion here:

  // Handle some boolean conditions.
  if (I->getType()->getPrimitiveSizeInBits() == 1) {
    assert(Preference == WantInteger && "One-bit non-integer type?");
    // X | true -> true
    // X & false -> false
    if (I->getOpcode() == Instruction::Or ||
        I->getOpcode() == Instruction::And) {
      PredValueInfoTy LHSVals, RHSVals;
  
      ComputeValueKnownInPredecessors(I->getOperand(0), BB, LHSVals,
                                      WantInteger, CxtI);
      ComputeValueKnownInPredecessors(I->getOperand(1), BB, RHSVals,
                                      WantInteger, CxtI);

In all other places, we seem to make just 1 recursive call and therefore no growth in width. In your particular case the obvious fix would be to just check that `operand(0) == operand(1)`, but I guess that it's easy enough to construct a test when they won't match with same effect, e.g.

  %x1 = or i1 %x0, %x0
  %x2 = or i1 %x1, %x1
  %x3 = or i1 %x1, %x2
  %x4 = or i1 %x2, %x3

Do you mind adding such test as well? Just to make sure that no one rules it out with a trivial partial fix.


Repository:
  rL LLVM

https://reviews.llvm.org/D54239





More information about the llvm-commits mailing list