[PATCH] D74075: [mlir][VectorOps] Introduce a `vector.fma` op that works on n-D vectors and lowers to `llvm.intrin.fmuladd`

[Nicolas Vasilache via Phabricator via llvm-commits]

nicolasvasilache marked 4 inline comments as done.
nicolasvasilache added inline comments.


================
Comment at: mlir/include/mlir/Dialect/VectorOps/VectorOps.td:384
+    pointwise multiply followed by accumulate. In the particular case of
+    lowering to LLVM, this is guaranteed to lower to the `llvm.fmuladd.*`
+    intrinsic.
----------------
fhahn wrote:
> IIRC llvm.fmuladd does not specify the rounding behaviour (mul and add may or may not be fused). 
> 
> The way I read the description here it seems like fusion is ecpexted for vector.fma unless I am missing something. It might be worth clarifying the rounding behaviour 
Thanks @fhahn ! I overlooked the semantics section that clearly states to use fma.
What I want is indeed fma, will adapt the revision accordingly.


================
Comment at: mlir/test/Conversion/VectorToLLVM/vector-to-llvm.mlir:614
+
+// CHECK-LABEL: llvm.func @vector_fma
+func @vector_fma(%a: vector<8xf32>, %b: vector<2x4xf32>)
----------------
aartbik wrote:
> note that something like
> 
> // CHECK-SAME: %[[A:arg[0-9]+]]: 
> 
> can be used if you want to match the argument inputs as well, see e.g.  insert_strided_slice3 (probably need to rename a/b into arg0/arg1 to be sure, although I think that renaming happens anyway)
I do not see this as necessary, there is a 1-1 mapping that is already captured by the type.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74075/new/

https://reviews.llvm.org/D74075