[PATCH] D103233: [Matrix] Preserve existing fast-math flags during lowering

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 27 03:03:17 PDT 2021


fhahn added a comment.

Thanks for the patch! Could you add test coverage for lowering of the other FP instructions (`fsub`, `fmul`, `fneg`)? And maybe also with some variations of individual FMFs.



================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:1154
                           IRBuilder<> &Builder, bool IsTiled,
-                          bool IsScalarMatrixTransposed) {
+                          bool IsScalarMatrixTransposed, Instruction *Inst) {
     const unsigned VF = std::max<unsigned>(
----------------
I think it would be cleaner to just pass in the FMF object here? We don't need the instruction for anything else.

Also, I think we don't need to separately pass in `AllowContraction` any longer?


================
Comment at: llvm/test/Transforms/LowerMatrixIntrinsics/preserve-existing-fast-math-flags.ll:16
+; CHECK-NEXT: fadd fast <2 x float>
+  %i1 = insertelement <4 x float> <float poison, float 0.000000e+00, float 0.000000e+00, float poison>, float %x, i64 0
+  %i2 = insertelement <4 x float> %i1, float %y, i64 3
----------------
Is there a reason we need the `insert element` instructions here, rather than passing in the second operand as vector to the function as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103233



More information about the llvm-commits mailing list