This is an archive of the discontinued LLVM Phabricator instance.

[mlir][llvm] Use TableGen to import compare ops from LLVM IR.
ClosedPublic

Authored by gysit on Oct 13 2022, 12:51 AM.

Details

Summary

The revision imports compare operations using TableGen generated
builders, instead of using the special handlers defined by the Importer.
It therefore adds a new llvmArgIndexes field that allows to specify
a mapping between MLIR argument and LLVM IR operand indexes if they do
not match. Additionally, the FCmp op is extended with an additional
builder and all compare operations are extended with verification
traits to ensure the operands types match. These extensions simplify
the logic of the newly introduced builders and are in line with the
compare operations define by the arithmetic dialect.

Diff Detail

Event Timeline

gysit created this revision.Oct 13 2022, 12:51 AM
gysit requested review of this revision.Oct 13 2022, 12:51 AM
ftynse accepted this revision.Oct 13 2022, 1:25 AM
ftynse added inline comments.
mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td
240

Ultra-nit

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
97–105

Nit: maybe we can overload LLVM::getVectorType to take llvm::ElementCount.

mlir/test/Target/LLVMIR/Import/instructions.ll
51

Any reason why this has to be on the next line? Otherwise I'd just CHECK. We also have a tendency to intersperse check lines and IR lines to improve the readability flow of long segments.

mlir/tools/mlir-tblgen/LLVMIRConversionGen.cpp
213–215

would llvm::append_range(llvmArgIndices, seq<int64_t>(...)) work here?

234

You may want to error out when a variadic operand is used.

This revision is now accepted and ready to land.Oct 13 2022, 1:25 AM
gysit added inline comments.Oct 13 2022, 1:31 AM
mlir/test/Target/LLVMIR/Import/instructions.ll
51

The intention was to make sure I did not miss a line. I will then switch back to interspersing check and ir for these long test functions!

ftynse added inline comments.Oct 13 2022, 1:35 AM
mlir/test/Target/LLVMIR/Import/instructions.ll
51

If you intersperse, you see that you haven't missed a line :)

gysit updated this revision to Diff 467418.Oct 13 2022, 3:04 AM
gysit marked 5 inline comments as done.

Address comments.