[PATCH] D91343: [InstCombine] Optimize away the unnecessary multi-use sign-extend

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:922
     // Compute the Known bits to simplify things downstream.
     computeKnownBits(I, Known, Depth, CxtI);
 
----------------
craig.topper wrote:
> lebedev.ri wrote:
> > Bhramar.vatsa wrote:
> > > For this patch, this call will no more happen, even when the 'case AShr' doesn't match completely. So, should a call to computeKnownBits(), be added in that case?
> > > The call isn't there for other cases, but there, perhaps it isn't needed.
> > Yes, this doesn't look immediately correct to me, but it's a preexisting problem.
> > I think the contents of `default:` switch should be located after the switch.
> > I think it would be an interesting experiment to make that change,
> > but not in this review.
> Don't the other handlers already do the equivalent of the default?
The equivalent being computing the `Known` - err, right, they do.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91343/new/

https://reviews.llvm.org/D91343