This is an archive of the discontinued LLVM Phabricator instance.

[mlir][vector] Implement Workaround Lowerings for Masked `fm**imum` Reductions
ClosedPublic

Authored by unterumarmung on Aug 24 2023, 1:29 PM.

Details

Summary

This patch is part of a larger initiative aimed at fixing floating-point max and min operations in MLIR: https://discourse.llvm.org/t/rfc-fix-floating-point-max-and-min-operations-in-mlir/72671.

Within LLVM, there are no masked reduction counterparts for vector reductions such as fmaximum and fminimum.
More information can be found here: https://github.com/llvm/llvm-project/issues/64940#issuecomment-1690694156.

To address this issue in MLIR, where we need to generate appropriate lowerings for these cases, we employ regular non-masked intrinsics.
However, we modify the input vector using the arith.select operation to effectively deactivate undesired elements using a "neutral mask value".
The neutral mask value is the smallest possible value for the fmaximum reduction and the largest possible value for the fminimum reduction.

Depends on D158618

Diff Detail

Event Timeline

unterumarmung created this revision.Aug 24 2023, 1:29 PM
Herald added a reviewer: ftynse. · View Herald Transcript
Herald added a reviewer: dcaballe. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
unterumarmung requested review of this revision.Aug 24 2023, 1:29 PM
unterumarmung edited the summary of this revision. (Show Details)Aug 24 2023, 1:34 PM

Rewrite to use only LLVM Dialect operations, without arith.constant, arith.select and vector.broadcast

unterumarmung edited the summary of this revision. (Show Details)Aug 25 2023, 10:45 AM
dcaballe accepted this revision.Aug 25 2023, 10:48 AM

LGTM! Just minor comments. You can address them before commiting. Thanks!

mlir/lib/Conversion/VectorToLLVM/ConvertVectorToLLVM.cpp
610

nit: Finish comment with '.' per coding standards. Here and a couple of times below.

633–634

nit: value -> neutralVal or something more specific.
nit: denseValue -> vecNeutral or something more specific.

This revision is now accepted and ready to land.Aug 25 2023, 10:48 AM