[PATCH] D39421: [InstCombine] Extracting common and-mask for shift operands of Or instruction

[Omer Paparo Bivas via Phabricator via llvm-commits]

opaparo added a comment.

In https://reviews.llvm.org/D39421#937705, @spatel wrote:

> I think inverting the canonicalization of shl+and would make your first test case optimize without this patch


Could you please explain why? I'm not sure I'm seeing it.

> Currently, we end up inverting the canonicalization in the x86 backend (because a smaller constant mask can be created in less instruction bytes), so it would be better to "get it right" here in IR in the first place.
>  I understand the multi-use case better now with your explanation, so I agree that we want this patch to handle those cases too. But I don't think we should ignore the underlying canonicalization choices just because we know we want to catch the larger patterns.

I agree that this alternative canonization could prove to be beneficial and more correct. However, I feel that this discussion is orthogonal to this patch, and if it would indeed be decided to switch to the new form then some of the code of this patch, along with several other pieces of code, may need to change accordingly.


https://reviews.llvm.org/D39421