[PATCH] D72820: [FPEnv] Add pragma FP_CONTRACT support under strict FP.

[Pengfei Wang via Phabricator via llvm-commits]

pengfei marked an inline comment as done.
pengfei added inline comments.


================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3381
+                             Addend->getType()),
+        {MulOp0, MulOp1, Addend, MulOp->getOperand(2), MulOp->getOperand(3)});
+  else
----------------
andrew.w.kaylor wrote:
> You shouldn't just assume that MulOp is a constrained intrinsic. Cast to ConstrainedFPIntrinsic and use ConstrainedFPIntrinsic::getRoundingMode() and ConstrainedFPIntrinsic::getExceptionBehavior(). The cast will effectively assert that MulOp is a constrained intrisic. I think that should always be true.
I prefer to reuse the operands from the fmul intrinsic here. 1). fmuladd always has the same exception/rounding mode with fmul. 2). the function getRoundingMode/getExceptionBehavior just return a enum value. We need more code to turn them into Value type.


================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3423
 
+  if (Builder.getIsFPConstrained()) {
+    if (auto *LHSBinOp = dyn_cast<llvm::CallBase>(op.LHS)) {
----------------
andrew.w.kaylor wrote:
> I don't think we should ever non-constrained create FMul instructions if Builder is in FP constrained mode, but you should assert that somewhere above. Maybe move this block above line 3409 and add:
> 
> assert(LHSBinOp->getOpcode() != llvm::Instruction::FMul && RHSBinOp->getOpcode() != llvm::Instruction::FMul);
Add assertion in line 3380. We only need to check once there.


================
Comment at: llvm/test/TableGen/GlobalISelEmitter-input-discard.td:18
 // GISEL-NEXT: GIM_CheckRegBankForClass, /*MI*/0, /*Op*/0, /*RC*/MyTarget::GPR32RegClassID,
-// GISEL-NEXT: // (intrinsic_w_chain:{ *:[i32] } 248:{ *:[iPTR] }, srcvalue:{ *:[i32] }, i32:{ *:[i32] }:$src1)  =>  (FOO:{ *:[i32] } (IMPLICIT_DEF:{ *:[i32] }), GPR32:{ *:[i32] }:$src1)
+// GISEL-NEXT: // (intrinsic_w_chain:{ *:[i32] } 249:{ *:[iPTR] }, srcvalue:{ *:[i32] }, i32:{ *:[i32] }:$src1)  =>  (FOO:{ *:[i32] } (IMPLICIT_DEF:{ *:[i32] }), GPR32:{ *:[i32] }:$src1)
 // GISEL-NEXT: GIR_MakeTempReg, /*TempRegID*/0, /*TypeID*/GILLT_s32,
----------------
It's strange the number is affected. I haven't found any cause.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72820