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

Mon Nov 9 05:28:45 PST 2020

spatel added a comment.

In D90734#2382164 <https://reviews.llvm.org/D90734#2382164>, @shchenz wrote:

> In D90734#2381468 <https://reviews.llvm.org/D90734#2381468>, @spatel wrote:
>
>> The medium-term solution (hopefully not too far away now) is to canonicalize to intrinsics - D87188 <https://reviews.llvm.org/D87188>. Once we get D90554 <https://reviews.llvm.org/D90554> in, we can try that canonicalization again.
>>
>> So I'm not sure that we want to add to the complexity of early-cse to handle this right now. What if we just delete the code that matches to SPF_ABS / SPF_NABS. I'm not seeing any regression test failures with that, and it avoids the bug?
>
> Thanks for your comments. I am fine to delete the SPF_ABS/SPF_NABS related codes in earlycse pass. In current opt pipeline, passes between the first earlycse and the first instcombine should not be sensitive to earlycse's result or will be rerun after instcombine? Please let me know if you need me to delete them.

We run earlycse at least once after instcombine in the typical optimization pipeline, so I don't think there is much chance for regression.

Can you update this patch to remove the SPF_ABS/SPF_NABS block and keep the new test to verify that we are not failing the hash matching?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90734