[PATCH] D90015: [InstSimpify][ValueTracking] Use new FP matchers

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 23 09:30:08 PDT 2020


lebedev.ri added a comment.

In D90015#2350426 <https://reviews.llvm.org/D90015#2350426>, @spatel wrote:

> In D90015#2349354 <https://reviews.llvm.org/D90015#2349354>, @foad wrote:
>
>> I split this out from D89038 <https://reviews.llvm.org/D89038> because of @lebedev.ri's comment:
>>
>>> I would strongly recommend to split this into two separate commits - adding the matchers, and using them.
>>> Because while the matchers themselves LG, i'm not at all confident with users.
>>> When doing latter, please ensure that each separate change has appropriate undef test coverage, and ideally check that alive2 is happy about those transforms.
>>
>> I haven't looked at adding the extra test coverage yet.
>
> Sorry, I'm overloaded with reviews, but I agree with @lebedev.ri that it was good to split this up. I'd go even further and split this patch up too and add tests for all known callers of the ValueTracking APIs. We've seen several cases of subtle bugs with partial-undef vectors, so we need more tests and ideally confirm correctness with Alive2.

+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90015



More information about the llvm-commits mailing list