[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