Page MenuHomePhabricator

[mlir][ArmSVE] Add masked arithmetic operations
ClosedPublic

Authored by jsetoain on Apr 21 2021, 11:08 AM.

Details

Summary

These instructions map to SVE-specific instrinsics that accept a
predicate operand to support control flow in vector code.

Diff Detail

Event Timeline

jsetoain created this revision.Apr 21 2021, 11:08 AM
jsetoain updated this revision to Diff 339345.Apr 21 2021, 12:21 PM

Add stronger type checking

jsetoain updated this revision to Diff 339539.Apr 22 2021, 2:40 AM
This comment was removed by jsetoain.
jsetoain updated this revision to Diff 340149.Apr 23 2021, 1:16 PM

Use classes to reduce redundancy in operation descriptions

jsetoain updated this revision to Diff 340150.Apr 23 2021, 1:24 PM

Add type checks

jsetoain published this revision for review.Apr 29 2021, 3:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 29 2021, 3:45 AM
Matt added a subscriber: Matt.May 3 2021, 5:38 AM
ftynse accepted this revision.May 5 2021, 12:37 AM

LGTM with comments addressed.

mlir/include/mlir/Dialect/ArmSVE/ArmSVE.td
387

Shouldn't this be commutative?

468

Nit: we can have type constraints stricter than AnyVector here

mlir/lib/Dialect/ArmSVE/IR/ArmSVEDialect.cpp
69

Nit: please add doc comment

74

I'd rather return null or assert here. We don't use NoneType to denote error state.

This revision is now accepted and ready to land.May 5 2021, 12:37 AM
jsetoain updated this revision to Diff 342976.May 5 2021, 3:04 AM
jsetoain marked 4 inline comments as done.

Address comments

mlir/include/mlir/Dialect/ArmSVE/ArmSVE.td
468

Since the LLVM Dialect already supports Scalable Vectors, this quick fix could go directly into LLVM dialect. To keep the patch contained within ArmSVE, and given that nobody else is using this yet, I think it's better to leave it here for now. Once the scalable vector has a more defined shape outside of ArmSVE, I myself will take care of adapting and moving these LLVM checks to where they belong. Alternatively, if this is not acceptable, I can create a different patch for LLVM dialect adding the extra type constraint, and make this patch depend on that one. That way, LLVM dialect changes don't get mixed up with hw-specific dialect changes.

ftynse accepted this revision.May 5 2021, 5:17 AM

You can follow this https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access to get commit access for yourself.

This revision was automatically updated to reflect the committed changes.