[PATCH] D41354: [InstCombine] Extending InstructionSimplify

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added inline comments.


================
Comment at: lib/Analysis/InstructionSimplify.cpp:3432
+  if (I->getOpcode() != Instruction::Shl)
+    // TODO: Perhaps we can handle other instructions too
+    return false;
----------------
I'd prefer not to have these kinds of TODO comment. If you have specific ideas, that's good to mention, but comments that are essentially "TODO: Think about this more.", don't add much value.

Comments should also end with a period.

In any case, with a bit of refactoring, I think that you could handle this in a more-general way. There definitely are other cases that could be handled. Note that the implementation in ValueTracking of known bits for an add/sub is essentially:

  unsigned BitWidth = KnownOut.getBitWidth();
  KnownBits LHSKnown(BitWidth);
  computeKnownBits(Op0, LHSKnown, Depth + 1, Q);
  computeKnownBits(Op1, Known2, Depth + 1, Q);
  KnownOut = KnownBits::computeForAddSub(Add, NSW, LHSKnown, Known2);

where the actual logic has been put in KnownBits::computeForAddSub (in lib/Support/KnownBits.cpp). To detect whether we can prove non-overflow, you could get the known bits of the operands, make the APInts larger, call KnownBits::computeForAddSub, and then see if the extended bits of the result are all known zero.

KnownBits::computeForAddSub is currently the only part of ValueTracking split out like that, but the same could be done for the code in computeKnownBitsMul and computeKnownBitsFromShiftOperator (or, more specifically, the relevant code in its caller) to handle mul and shifts.


Repository:
  rL LLVM

https://reviews.llvm.org/D41354