[PATCH] D143919: [Clang] Copy strictfp attribute from pattern to instantiation

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D143919#4130178 <https://reviews.llvm.org/D143919#4130178>, @efriedma wrote:

> If the "strictfp" attribute is based on the contents of the function body, should we recompute it when the function is instantiated, as opposed to copying it?  For example, say you have a pragma inside an "if constexpr"; should that set the strictfp attribute if it's discarded?
>
> Otherwise, I guess updateAttrsForLateParsedTemplate makes sense.

Recomputing is certainly a better solution, as it would provide actual properties. It however should be applied to all instantiations, not only late parsed. With this change the code:

  template <typename T, bool F>
  T templ_01(T x, T y) {
    if constexpr (F) {
      #pragma STDC FENV_ACCESS ON
      return x - y;
    } else
    return x + y;
  }
  
  float func_02(float x, float y) {
    return templ_01<float, true>(x, y);
  }
  
  float func_03(float x, float y) {
    return templ_01<float, false>(x, y);
  }

compiled without late parsing produces `func_03` with strictfp attribute. However constarined intrinsics in it are semantically equivalent to non-constrained operations. Adding unneeded strictfp attribute is not an error.



================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:824
+      continue;
+    }
+
----------------
efriedma wrote:
> Is this necessary?  The non-delayed-parsed case seems to work correctly on trunk without any changes, so I suspect the autogenerated sema::instantiateTemplateAttribute is doing the right thing.
Yes, it is necessary. Without it the code from the added test crashes, as constrained intrinsic is used in a function without strictfp attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143919