[PATCH] D93860: [SLP] delete unused pairwise reduction option

Pengfei Wang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 30 07:30:10 PST 2020


pengfei added a comment.

In D93860#2475028 <https://reviews.llvm.org/D93860#2475028>, @spatel wrote:

> In D93860#2473348 <https://reviews.llvm.org/D93860#2473348>, @pengfei wrote:
>
>> Hi Sanjay, pairwise reduction seems not adopted by any targets. But do we need to consider the FMF here? I found LangRef says the intrinsic is sequential for fadd and fmul if `reassoc` flag is not set.
>
> Hi @pengfei - 
> Sorry for the delayed reply - I did not get an email notification for your comment here in Phab.
>
> We do not need to care about FMF directly here because neither of these shuffle variations corresponds to "sequential" reduction of the elements. In other words, we should require `reassoc` (at the least) to form an fadd or fmul reduction that is expected to need any shuffles. Let me know if you see any holes in that theory.
>
> One motivation for this cleanup is to correct bugs with using/propagating FMF (that was also true for D87416 <https://reviews.llvm.org/D87416>). For example - https://llvm.org/PR35538

Thanks for explanation. I was confused it with reduction intrinsics. I noticed `tryToReduce` sets fast flag instead of requires. Is that the problem you mentioned? By the way, can we build reduction intrinsics directly here instead of doing shuffle?


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

https://reviews.llvm.org/D93860



More information about the llvm-commits mailing list