pengfei marked an inline comment as done. pengfei added inline comments.
================ Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3381 + "constrained mode"); + FMulAdd = Builder.CreateCall( + CGF.CGM.getIntrinsic(llvm::Intrinsic::experimental_constrained_fmuladd, ---------------- kpn wrote: > craig.topper wrote: > > kpn wrote: > > > craig.topper wrote: > > > > Doesn't this need to be CreateConstrainedFPCall so that the strictfp > > > > attribute is added? That will take care of adding the metadata operands > > > > too. > > > Is this code tested? I ran into a bug yesterday where CreateCall was used > > > with a constrained intrinsic and the Instruction class blew up because > > > the function signature was wrong. I wasn't passing in the metadata > > > arguments. > > > > > > So, yes, it should be, and it would might make sense for the patch to > > > have test coverage that catches any other cases of this. > > This code is copying the metadata arguments from the fmul intrinsic, MulOp. > > that’s the getOperand(2) and getOperand(3). > Ah, yes, thanks. Your comment about the attribute is still valid, though. > And, yes, using CreateConstrainedFPCall() is the easiest way to fix the > attribute. Yes, it's the best choice. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72820/new/ https://reviews.llvm.org/D72820 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
