Page MenuHomePhabricator

[mlir][llvm] Add support for importing masked intrinsics from LLVM IR.
ClosedPublic

Authored by gysit on Oct 17 2022, 12:20 AM.

Details

Summary

The revision adds support for importing the masked load/store and
gather/scatter intrinsics from LLVM IR. To enable the import, the
revision also includes an extension of the mlirBuilder code generation
to support variadic arguments.

Depends on D136057

Diff Detail

Event Timeline

gysit created this revision.Oct 17 2022, 12:20 AM
gysit requested review of this revision.Oct 17 2022, 12:20 AM
ftynse added inline comments.Oct 17 2022, 12:47 AM
mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
247

Why does this have to materialize the vector?

mlir/tools/mlir-tblgen/LLVMIRConversionGen.cpp
80

Be aware that isVariableLength checks for both variadic and optional.

gysit added inline comments.Oct 17 2022, 1:04 AM
mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
247

True I can return an ArrayRef.

mlir/tools/mlir-tblgen/LLVMIRConversionGen.cpp
80

I think we want to allow both at the moment but this is indeed a bit confusing given the name of the method. There are certain operations that use Optional<..> and certain others use Variadic<...>, which makes sense since the ReturnOp either takes 0 or 1 argument.

Would you prefer to support only Variadic operands here and handle Optional<...> in a different way. I can try how many things break if I am more restrictive.

gysit updated this revision to Diff 468132.Oct 17 2022, 1:28 AM
gysit marked 2 inline comments as done.

Address comments.

ftynse accepted this revision.Oct 17 2022, 4:26 AM
ftynse added inline comments.
mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
247

At this point, do you really need this helper? :)

mlir/tools/mlir-tblgen/LLVMIRConversionGen.cpp
80

Yes, I prefer with start with being more strict. Optional operands are more frequent than variadic ones and we would better be on the safe side given we are out of the tested path here.

This revision is now accepted and ready to land.Oct 17 2022, 4:26 AM
gysit added inline comments.Oct 17 2022, 4:37 AM
mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp
247

I'll drop it yes.

gysit updated this revision to Diff 468170.Oct 17 2022, 5:28 AM

Address comments.