[PATCH] D90734: [EarlyCSE] make abs recognization not depend on instcombine abs canonicalize

Tue Nov 10 07:07:59 PST 2020

shchenz added a comment.

In D90734#2385582 <https://reviews.llvm.org/D90734#2385582>, @spatel wrote:

> In D90734#2385581 <https://reviews.llvm.org/D90734#2385581>, @shchenz wrote:
>
>> In D90734#2385565 <https://reviews.llvm.org/D90734#2385565>, @shchenz wrote:
>>
>>> I am thinking `llvm.abs(A)` and `llvm.abs(-A)` can not be optimized in current earlyCSE phase? There are logics for commutative intrinsics and intrinsics with same operands in EarlyCSE pass. Am I missing something?
>>
>> Ah, I know your ideas ^-^. Instcombine should canonicalize `llvm.abs(-A)` to `llvm.abs(A)`. So after instcombine, in earlycse, we will not see `llvm.abs(-A)` right? I don't have other concerns. Thanks.
>
> Right - I do not think we should put any extra pattern matching logic into this pass unless it is very simple. Everything else is dangerous - as this bug/patch (and a few before it) have already shown. :)
> LGTM.

Thank you!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90734