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

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 17 12:44:45 PST 2020


lebedev.ri added a comment.

Please do say if you need help landing this (state the `name e at ma.il` to be used for the git commit's `Author` field).



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:922
     // Compute the Known bits to simplify things downstream.
     computeKnownBits(I, Known, Depth, CxtI);
 
----------------
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.


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

https://reviews.llvm.org/D91343



More information about the llvm-commits mailing list