This patch removed duplicate code for matching patterns which are not handled in SimplifyUsingDistributiveLaws() (after r211261)
Looks good in general. I am reading this patch out of context. Are you removing these because you added similar logic in InstCombine? A further question I am curious about is what's the split between InstructionSimplify and InstCombine. Both seem to simplify instructions. What should be in one versus the other?
Yes, With SimplifyUsingDistributiveLaws() updated in r211261, now it can handle all transforms
handled by these removed code. This patch removes code duplication. I am planning to further
analyse transform using associative, commutative and distributive laws and generalize them.
I had doubt about removing code from instruction simplify as it works as independent pass. I have
talked to Ben before submitting this patch for review.
I do have my doubts about what goes to simplify and what in combine. But from what I have read
in code and mailing list, if we can transform instruction to one of its operand or to some simple
value, it should be in simplify. Instruction combine has transforms which combine 2 or more
instruction to less instruction(s).
Thanks for clarification! It makes much sense to me now. Some nits, otherwise LGTM.
|36 ↗||(On Diff #10744)|
Could you add a comment that translates the IR into math expressions? Just as what you did for factorize1|2|3. It would be much easier to follow. Thanks!
|48 ↗||(On Diff #10744)|