Page MenuHomePhabricator

[mlir] [VectorOps] Add the ability to mark FP reductions with "reassociate" attribute

Authored by aartbik on Jun 25 2020, 10:41 PM.



In general, passing "fastmath" from MLIR to LLVM backend is not supported, and even just providing such a feature for experimentation is under debate. However, passing fine-grained fastmath related attributes on individual operations is generally accepted. This CL introduces an option to instruct the vector-to-llvm lowering phase to annotate floating-point reductions with the "reassociate" fastmath attribute, which allows the LLVM backend to use SIMD implementations for such constructs. Oher lowering passes can start using this mechanism right away in cases where reassociation is allowed.

For some microbenchmarks on x86-avx2, speedups over 20 were observed for longer vector (due to cleaner, spill-free and SIMD exploiting code).

mlir-opt --convert-vector-to-llvm="reassociate-fp-reductions"

Diff Detail

Event Timeline

aartbik created this revision.Jun 25 2020, 10:41 PM
Herald added a project: Restricted Project. · View Herald Transcript
ftynse requested changes to this revision.Jun 26 2020, 12:58 AM
ftynse added inline comments.

I'm generally reluctant to having LLVM dialect operations that do don't exist in LLVM IR, as either operations or intrinsics. Could we rather add attributes to the already existing LLVM_VectorReductionV2 for reassociation (and "fast" since there are only two that need to be supported). This should be as simple as

Arguments<(ins LLVM_Type, LLVM_Type, DefaultValuedAttr<BoolAttr, "false">:$reassoc, DefaultValuedAttr<BoolAttr, "false">:$fast)>

and using them in the llvmBuilder. When we have a general mechanism for fast-math flags, we can rely on that instead.

This revision now requires changes to proceed.Jun 26 2020, 12:58 AM
aartbik marked an inline comment as done.Jun 26 2020, 9:11 AM
aartbik added inline comments.

Thanks. Agreed an attributed is cleaner. Changed that (note that I just need $reassoc, not $fast ). I had an internal prototype for passing on fast-math flags in general, but that stopped after an internal discussion, since it was deemed to dangerous. Hence the intrinsic focused solution for now.

aartbik updated this revision to Diff 273773.Jun 26 2020, 10:16 AM

used attributed rather than naming for reassoc=true

aartbik edited the summary of this revision. (Show Details)Jun 26 2020, 10:17 AM
aartbik edited the summary of this revision. (Show Details)
ftynse accepted this revision.Jun 26 2020, 10:19 AM


This revision is now accepted and ready to land.Jun 26 2020, 10:19 AM
mehdi_amini accepted this revision.Jun 26 2020, 11:01 AM

Nice! I agree the attribute is appropriate here.

To be clear: I'm not against a fast-math for experimenting, I wanted to make sure we build what you did here as a "proper" solution and we don't rely on the "big hammer" global fast-math instead.

This revision was automatically updated to reflect the committed changes.