[PATCH] D40649: [InstCombine] Don't crash on out of bounds shifts

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D40649#942228, @igor-laevsky wrote:

> I see your point. Unfortunately I can't find any good place to common this out. Problem is that code is structured as a consecutive matching of unrelated patterns which may sometimes contain invalid shift operations. But I don't know the code base enough so maybe I'm missing something. Any advice?


Not sure if you saw my earlier patch draft. Here's a hopefully better version - does this solve the crashing you're seeing?

  Index: lib/Analysis/ValueTracking.cpp
  ===================================================================
  --- lib/Analysis/ValueTracking.cpp	(revision 319542)
  +++ lib/Analysis/ValueTracking.cpp	(working copy)
  @@ -800,7 +800,16 @@
     unsigned BitWidth = Known.getBitWidth();
   
     if (auto *SA = dyn_cast<ConstantInt>(I->getOperand(1))) {
  -    unsigned ShiftAmt = SA->getLimitedValue(BitWidth-1);
  +    // We can't use getZExtValue() here because the shift amount could be bigger
  +    // than a 64-bit, but anything over 32-bit (the knownbits width is
  +    // 32-bit) would result in poison, so it's fine to saturate to a 64-bit.
  +    uint64_t ShiftAmt = SA->getLimitedValue();
  +    if (ShiftAmt >= BitWidth) {
  +      // This shift produces poison because the shift amount is too big. We can
  +      // return anything we want. Choose 0 for the best folding opportunity.
  +      Known.setAllZero();
  +      return;
  +    }
   
       computeKnownBits(I->getOperand(0), Known, Depth + 1, Q);
       Known.Zero = KZF(Known.Zero, ShiftAmt);


https://reviews.llvm.org/D40649