[PATCH] D69798: Implement inlining of strictfp functions

[Andy Kaylor via Phabricator via llvm-commits]

andrew.w.kaylor added inline comments.


================
Comment at: llvm/include/llvm/IR/Instruction.h:50
+    /// Indicates that this instruction depends on floating point environment.
+    DependsOnFPEnvironment = 1 << 14,
     /// This is a bit stored in the SubClassData field which indicates whether
----------------
It seems wasteful to take one of these bits for something that can be deduced from other information we already have.


================
Comment at: llvm/lib/Transforms/Utils/CloneFunction.cpp:310
+  Instruction *NewInst;
+  if (HostFuncIsStrictFP && OldInst.dependsOnFPEnvironment()) {
+    // Instead of cloning the instruction, a call to constrained intrinsic
----------------
There is some ongoing discussion about how predicated vector instructions will handle constrained FP mode. I think Simon's intention is to have just a single intrinsic that is used regardless of whether strictfp semantics are needed.

Also, in addition to converting fp intrinsics to strict equivalents, I think we need to add the strictfp attribute to callsites. We're currently preventing libcall simplification for callsites marked strictfp. The plan has been for front ends to mark all calls as strictfp so that they don't need to identify math library calls. I could probably be convinced that this is not necessary. Arguably, simplifyLibCalls could look at the callee's function attributes instead. @kpn has been considering whether this behavior should be relaxed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69798