[PATCH] D80124: [Matrix] Make matrix.multiply variadic, accept optional NUW/NSW flags.
Florian Hahn via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue May 19 04:49:46 PDT 2020
fhahn marked 3 inline comments as done.
fhahn added inline comments.
================
Comment at: llvm/docs/LangRef.rst:15198
declare vectorty @llvm.matrix.multiply.*(vectorty %A, vectorty %B, i32 <M>, i32 <N>, i32 <K>)
+ declare vectorty @llvm.matrix.multiply.*(vectorty %A, vectorty %B, i32 <M>, i32 <N>, i32 <K>, i1 <HasNUW>, i1 <HasNSW>)
----------------
LuoYuanke wrote:
> It reminds me that for float type we may have more flags to support. Do we need to have a strict FP version and fast FP version for it? Here is link of fast-math flags. https://llvm.org/docs/LangRef.html#fast-math-flags.
> It reminds me that for float type we may have more flags to support. Do we need to have a strict FP version and fast FP version for it? Here is link of fast-math flags. https://llvm.org/docs/LangRef.html#fast-math-flags.
FMF can be specified directly at the call site and it is already used for selecting fmuladd (https://github.com/llvm/llvm-project/blob/master/llvm/test/Transforms/LowerMatrixIntrinsics/multiply-double-contraction-fmf.ll#L65). Other flags FMF should probably be passed through directly and there should be no need to use extra arguments for that.
For the constrained FP math mentioned by @tschuett we would need extra arguments to indicate the constrained versions should be used. This should be relatively straight-forward to do, but I probably won't have time to work on this in the next few months at least.
================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:936
+ IRBuilder<> &Builder, bool AllowContraction, bool HasNUW,
+ bool HasNSW, unsigned &NumComputeOps) {
NumComputeOps += getNumOps(A->getType());
----------------
nicolasvasilache wrote:
> has this reached a point where you want a separate `LowerMatrixIntrinisicsOptions().setHasNUW(true).setHasNSW(false)`and make this into
> `createMulAdd(Value *Sum, Value *A, Value *B, LowerMatrixIntrinisicsOptions options)` ?
>
> As a future user of the potential per-operand row/col major arguments it would make things much nicer IMO.
yeah, I've added a `MultiplyOptions` struct. Should help in the future, also with other fast-math flags/constrained math options.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80124/new/
https://reviews.llvm.org/D80124
More information about the llvm-commits
mailing list