[PATCH] D66092: [CodeGen] Generate constrained fp intrinsics depending on FPOptions

Andy Kaylor via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 15 14:42:35 PDT 2019


andrew.w.kaylor added a comment.

In D66092#1630997 <https://reviews.llvm.org/D66092#1630997>, @sepavloff wrote:

> Replacement of floating point operations with constrained intrinsics seems more an optimization helper then a semantic requirement. IR where constrained operations are mixed with unconstrained is still valid in sense of IR specification.


The thing that makes the IR semantically incomplete is that there is nothing there to prevent incorrect code motion of the non-constrained operations. Consider this case:

  if (someCondition) {
    #pragma clang fp rounding(downward)
    fesetround(FE_DOWNWARD);
    x = y/z;
    fesetround(FE_TONEAREST);
  }
  a = b/c;

If you generate a regular fdiv instruction for the 'a = b/c;' statement, there is nothing that would prevent it from being hoisted above the call to fesetround() and so it might be rounded incorrectly.

In D66092#1630997 <https://reviews.llvm.org/D66092#1630997>, @sepavloff wrote:

> Another issue is non-standard rounding. It can be represented by constrained intrinsics only. The rounding does not require restrictions on code motion, so mixture of constrained and unconstrained operation is OK. Replacement of all operations with constrained intrinsics would give poorly optimized code, because compiler does not optimize them. It would be a bad thing if a user adds the pragma to execute a statement with specific rounding mode and loses optimization.


I agree that loss of optimization would be a bad thing, but I think it's unavoidable. By using non-default rounding modes the user is implicitly accepting some loss of optimization. This may be more than they would have expected, but I can't see any way around it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66092





More information about the cfe-commits mailing list