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

[Craig Topper via Phabricator via llvm-commits]

craig.topper 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);
 
----------------
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?


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

https://reviews.llvm.org/D91343