[PATCH] D112117: [llvm][LV] Drop poison-generating flags from FP instructions

Fri Dec 10 03:20:10 PST 2021

dcaballe added a subscriber: nlopes.
dcaballe added a comment.

In D112117#3183418 <https://reviews.llvm.org/D112117#3183418>, @reames wrote:

> JFYI, I've been taking a look at trying to get FMF flags into the generic dropPoisonGeneratingFlags.  So far, the test diff (which looks a lot like this one for the same reasons) has exposed a bunch of cases where the existing code had problems.  I'm working through fixing them individually, with the goal of getting a reasonable diff.
>
> The most relevant one which got some discussion on this review is the reduction expansion.  That appears to be entirely an artifact of code which a) does nothing without the FMF handling in the generic code, and b) serves no purpose even with it.  0d13f94c <https://reviews.llvm.org/rG0d13f94c1da994e5ad6cf18206d6ca2f6056ef02> and b24db85 <https://reviews.llvm.org/rGb24db85c0b13cfb2f8e5a9f7ed445cdc7f05afd6> simply remove most of it, and fix up some comments.

Thanks for the notice, Philip. Based on the feedback that @nlopes provided in D111846 <https://reviews.llvm.org/D111846>, we wouldn't need to drop FMF as part of the vectorization process. That is why I abandoned this review. Maybe you could do something similar to what was suggested above:

> We could enable dropping FMF only with an optional bool parameter?

and make sure that the flag is disabled in the vectorizer to preserve what I understand is the expected behavior?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112117