[PATCH] D103233: [Matrix] Preserve existing fast-math flags during lowering
Florian Hahn via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri May 28 04:12:14 PDT 2021
fhahn added a comment.
Thanks for the update! A few more suggestions about dealing with the `FMF` object.
================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:407
+private:
+ static Optional<FastMathFlags> getFastMathFlags(Instruction *Inst) {
+ if (isa<FPMathOperator>(*Inst)) {
----------------
Instead of returning `None`, could we just return a FastMathFlags object with all options set the false (which is the default) ?
================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:1183
+ Builder.setFastMathFlags(FMF);
+ AllowContraction |= FMF.allowContract();
+ }
----------------
I think this should be the other way around, if `AllowContractEnabled` we could set `AllowContraction` on `FMF`. This default could be set in `getFastMathFlags`?
================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:1689
case Instruction::FAdd:
- return Builder.CreateFAdd(LHS, RHS);
+ return Builder.CreateFAddFMF(LHS, RHS, Inst);
case Instruction::FMul:
----------------
Can we instead use the FMF object in those versions instead? `AllowContraction` option should probably also add the `contract` flag to those.
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