This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Add mlir::LLVM::FastmathFlags to LLVM instrinsic vector reductions
ClosedPublic

Authored by bmyerz on Mar 9 2023, 7:51 AM.

Details

Summary

Rationale:
The LLVM dialect supports passing fastmath flags from floating point ops to LLVMIR instructions. However, not all LLVM ops have the required attribute. This change adds support for fastmath flags to llvm.intr.vector.reduce.{fmin,fmax}. One scenario where this is useful is in lowering llvm.intr.vector.reduce.{fmax,fmin} to LLVMIR with nnan (NoNans) flag so it may be lowered to a shuffle reduction.

Changes:

  • Make LLVM_VecReductionF implement the FastmathFlagsInterface; change is modeled on LLVM_UnaryIntrOpF
  • Add an assembly format for LLVM_VecReductionF ops. The purpose is to keep existing functionality: avoid printing the fastmath flags attribute when it has its default value (none). Change is modeled on LLVM_UnaryIntrOpBase

Diff Detail

Event Timeline

bmyerz created this revision.Mar 9 2023, 7:51 AM
Herald added a project: Restricted Project. · View Herald Transcript
bmyerz requested review of this revision.Mar 9 2023, 7:51 AM
Herald added a project: Restricted Project. · View Herald Transcript
bmyerz updated this revision to Diff 503768.Mar 9 2023, 7:53 AM

remove unintended .arcconfig change

bmyerz edited the summary of this revision. (Show Details)Mar 9 2023, 8:02 AM
bmyerz edited the summary of this revision. (Show Details)Mar 9 2023, 8:07 AM
bmyerz edited the summary of this revision. (Show Details)Mar 9 2023, 8:11 AM
bmyerz edited the summary of this revision. (Show Details)
gysit added a comment.Mar 9 2023, 12:53 PM

Thanks!

LGTM modulo some minor comments.

mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td
389

nit: drop spaces

404

nit: drop spaces

mlir/test/Dialect/LLVMIR/roundtrip.mlir
520–523

ultra nit: it is slightly preferred if the CHECK lines are directly above the tested IR to improve readability.

mlir/test/Target/LLVMIR/llvmir-intrinsics.mlir
257–260

It seems the fastmath flags lowering is already tested in llvmmir.mlir (@fastmathFlags)? I would thus drop the two tests added here.

bmyerz updated this revision to Diff 503926.Mar 9 2023, 1:55 PM
bmyerz edited the summary of this revision. (Show Details)

address review comments on 1. whitespace, 2. placement of CHECK, 3. redundant test

bmyerz marked 3 inline comments as done.Mar 9 2023, 1:55 PM
bmyerz marked an inline comment as done.
gysit accepted this revision.Mar 9 2023, 11:02 PM

Great thanks!

This revision is now accepted and ready to land.Mar 9 2023, 11:02 PM