[PATCH] D72733: [InstCombine] allow more narrowing of casted select

Wed Jan 15 09:45:07 PST 2020

bjope added a comment.

Thanks for looking into this. I did not know about `shouldChangeType` but that seems like a useful function here.

Slightly worried just like @lebedev.ri that we might swing back into getting a regression for some other code pattern. But it seems like it at least doesn't hit any regressions for existing lit-tests, so it doesn't regress anything that we have optimized (and added tests for) earlier at least.

Btw, D47163 <https://reviews.llvm.org/D47163> landed 1.5 years ago, so we have had the regression from PR44545 for awhile. So even if it would be nice to fix it right away, probably no need to rush into something that we do not strongly believe in.

Anyway, the patch LGTM. But let's see if @lebedev.ri has any examples showing that this might a bad idea.

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

https://reviews.llvm.org/D72733