Page MenuHomePhabricator

[mlir][llvm] Tighten LLVM dialect intrinsic type constraints.
ClosedPublic

Authored by gysit on Oct 20 2022, 7:46 AM.

Details

Summary

The revision specifies more precise argument and result type
constraints for many of the llvm intrinsics. Additionally, add
tests to verify intrinsics with invalid arguments/result result
in a verification error.

Diff Detail

Event Timeline

gysit created this revision.Oct 20 2022, 7:46 AM
Herald added a project: Restricted Project. · View Herald Transcript
gysit requested review of this revision.Oct 20 2022, 7:46 AM
ftynse accepted this revision.Oct 25 2022, 2:06 AM
ftynse added inline comments.
mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td
22

I suspect we want AnySignlessInteger here and below, MLIR integers can have signs and we don't want those in the LLVM intrinsics.

mlir/test/Target/LLVMIR/llvmir-invalid.mlir
138

Nit: FileCheck now supports @below instead of @+1 that reads better and works when combining errors and notes.

This revision is now accepted and ready to land.Oct 25 2022, 2:06 AM
gysit updated this revision to Diff 470450.Oct 25 2022, 5:44 AM

Address comments:

  • use @below instead of @+1
  • use AnySignlessInteger for all intrinsics

Rebase (after https://reviews.llvm.org/D136498):

  • use qualified(type($ptr)) for arguments of type LLVM_AnyPointer
  • adapt expected error messages
gysit marked 2 inline comments as done.Oct 25 2022, 5:50 AM

@dcaballe I changed the integer type constraints on the vector predicate intrinsics from AnyInteger to AnySignlessInteger as well. I hope this does not conflict with your changes.

@dcaballe I changed the integer type constraints on the vector predicate intrinsics from AnyInteger to AnySignlessInteger as well. I hope this does not conflict with your changes.

That should be fine! Thanks for pinging me :)