Page MenuHomePhabricator

[mlir] Fix LLVM intrinsic convesion generator for overloadable types.
ClosedPublic

Authored by kariddi on Jan 17 2020, 7:32 PM.

Details

Summary

If an intrinsic has overloadable types like llvm_anyint_ty or
llvm_anyfloat_ty then to getDeclaration() we need to pass a list
of the types that are "undefined" essentially concretizing them.

This patch add support for deriving such types from the MLIR op
that has been matched.

Diff Detail

Event Timeline

kariddi created this revision.Jan 17 2020, 7:32 PM
rriddle requested changes to this revision.Jan 17 2020, 7:40 PM
rriddle added inline comments.
mlir/tools/mlir-tblgen/LLVMIRIntrinsicGen.cpp
80

Remove all static functions from namespace scope.

92

Use camelCase for variable names.

179

Use cast if the result isn’t being check for null.

This revision now requires changes to proceed.Jan 17 2020, 7:40 PM
rriddle added inline comments.Jan 17 2020, 7:41 PM
mlir/tools/mlir-tblgen/LLVMIRIntrinsicGen.cpp
71

Function/top-level comments should be triple tick.

92

This applies to the rest of this revision as well.

kariddi updated this revision to Diff 238941.Jan 17 2020, 7:45 PM

Testing the change by adapting existing test

Unit tests: pass. 61991 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

kariddi updated this revision to Diff 238942.Jan 17 2020, 8:06 PM

Thanks River for the review. Addressed your comments.

kariddi updated this revision to Diff 238943.Jan 17 2020, 8:07 PM

Removing space

Unit tests: pass. 61991 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Unit tests: pass. 61991 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Harbormaster completed remote builds in B44338: Diff 238943.

Unit tests: pass. 61991 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

kariddi marked 4 inline comments as done.Jan 18 2020, 1:19 AM
rriddle added inline comments.Jan 19 2020, 1:01 PM
mlir/test/mlir-tblgen/llvm-intrinsics.td
9

Start sentences with a capital letter.

mlir/tools/mlir-tblgen/LLVMIRIntrinsicGen.cpp
35

nit: Can you just use a SmallBitVector instead?

37

nit: Remove this first line.

178

Can you use getOperand instead?

kariddi updated this revision to Diff 239223.Jan 20 2020, 7:03 PM
kariddi marked 4 inline comments as done.

Fixing additional comments.

kariddi added inline comments.Jan 20 2020, 7:08 PM
mlir/tools/mlir-tblgen/LLVMIRIntrinsicGen.cpp
178

Thanks, good one! I actually missed that we had getOperand() available

Unit tests: pass. 62041 tests passed, 0 failed and 783 were skipped.

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

rriddle accepted this revision.Jan 21 2020, 10:48 AM
rriddle added inline comments.
mlir/tools/mlir-tblgen/LLVMIRIntrinsicGen.cpp
35

Please add a comment for this using directive.

168

nit: Swap these two variables to match the loop order below.

181

These !empty() checks need to be changed to 'any' now.

This revision is now accepted and ready to land.Jan 21 2020, 10:48 AM
kariddi updated this revision to Diff 239391.Jan 21 2020, 11:32 AM
kariddi marked 3 inline comments as done.

Addressing comments

Comments addressed

Unit tests: fail. 62078 tests passed, 3 failed and 784 were skipped.

failed: Clang.Driver/cc-print-options.c
failed: LLVM.Transforms/ConstProp/fma.ll
failed: LLVM.Transforms/InstSimplify/fp-nan.ll

clang-tidy: unknown.

clang-format: pass.

Build artifacts: diff.json, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

This revision was automatically updated to reflect the committed changes.