Page MenuHomePhabricator

[MLIR]Add support for Arith MAX & MIN operations
ClosedPublic

Authored by Leporacanthicus on Aug 15 2022, 11:32 AM.

Details

Summary

There are some of this supported in various places, but the
basic conversion of single operations to LLVM was not supported.

Adding this to allow Flang to use these.

Diff Detail

Event Timeline

Leporacanthicus requested review of this revision.Aug 15 2022, 11:32 AM
mlir/test/Conversion/ArithmeticToLLVM/arith-to-llvm.mlir
389–409

Do we have to test the vector version as well?
Do you know why intrinsics are used in the conversion instead of regular llvm instructions like llvm.minimum.*? Also, this seems to be the first place where llvm.intr instructions are used.

bondhugula accepted this revision.Aug 16 2022, 6:57 PM
bondhugula added a subscriber: bondhugula.
bondhugula added inline comments.
mlir/lib/Conversion/ArithmeticToLLVM/ArithmeticToLLVM.cpp
337–342

Not sure why this list was not sorted earlier but it'll be good to add these in sorted order.

mlir/test/Conversion/ArithmeticToLLVM/arith-to-llvm.mlir
391–406

Consider avoiding numbered SSA values in CHECK lines - although the use/scope is a bit limited here.

This revision is now accepted and ready to land.Aug 16 2022, 6:57 PM
This revision was automatically updated to reflect the committed changes.
mlir/lib/Conversion/ArithmeticToLLVM/ArithmeticToLLVM.cpp
337–342

I will submit a patch to get all of them in sorted order.

mlir/test/Conversion/ArithmeticToLLVM/arith-to-llvm.mlir
391–406

I was following the pattern in the test on the lines above this one.

It seems like most of the rest of the tests are also using direct numbers.

I'll take a look and see if I can fix up the whole lot.

mlir/lib/Conversion/ArithmeticToLLVM/ArithmeticToLLVM.cpp
337–342

Sorting the table (and the declarations a few lines further up):
https://reviews.llvm.org/D132046