[Clang] Reset FP options before function instantiations

[sepavloff]

Summary

Previously function template instantiations occurred with FP options
that were in effect at the end of translation unit. It was a problem
for late template parsing as these FP options were used as attributes of
AST nodes and may result in crash. To fix it FP options are set to the
state of the point of template definition.

[efriedma]

Using the FPOptions from the beginning of the file doesn't seem much better than the FPOptions at the end of the file. Don't we want to use the FPOptions from the point of definition of the template?

[sepavloff]

Use FPOptions from the place where template is defined rather than default

[sepavloff]

Ping.

[efriedma]

LGTM

[rjmccall]

clang/lib/Parse/ParseTemplate.cpp

1747

Ah, is this bug specific to the MSVC-compatible template logic?

[sepavloff]

clang/lib/Parse/ParseTemplate.cpp

1747

Exactly. This is MSVC bug-or-feature.

[rjmccall]

Hmm. Why are we clearing the FP pragma stack instead of saving the old context onto it and then restoring after instantiation? I don't think semantic analysis ever depends on enclosing members of the stack, does it?

Clearing the entire stack might not matter much if we're at the end of the translation unit, which is the normal time to instantiate things, but it would matter if we're eagerly instantiating within the translation unit, which we have to do for various reasons, including explicit instantiation and constexpr.

[sepavloff]

In D143241#4441812, @rjmccall wrote:

Hmm. Why are we clearing the FP pragma stack instead of saving the old context onto it and then restoring after instantiation? I don't think semantic analysis ever depends on enclosing members of the stack, does it?

It shouldn't but the function body may contain pragmas and in the case of errors we could have weird behavior. Anyway, parsing in this case occurs at the end of translation unit, so it should be safe to clear pragma stack.

Clearing the entire stack might not matter much if we're at the end of the translation unit, which is the normal time to instantiate things, but it would matter if we're eagerly instantiating within the translation unit, which we have to do for various reasons, including explicit instantiation and constexpr.

Explicit instantiation performs late parsing at the end of translation unit, just as implicit one. Constexpr functions are parsed as usualy, at the point of definition (see Sema::canDelayFunctionBody). It seems late parsing always occurs at the end of TU.

[Jake-Egan]

Hi, the following assertion is getting tripped on AIX:

Assertion failed: *FpPragmaCurrentValue == SemaObj->FpPragmaStack.DefaultValue && "Expected a default pragma float_control value", file  /scratch/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/clang/lib/Serialization/ASTReader.cpp, line 8150, void clang::ASTReader::UpdateSema()()

https://lab.llvm.org/buildbot/#/builders/214/builds/8309/steps/6/logs/FAIL__Clang__late-parsed-instantiations_cpp

[sepavloff]

In D143241#4475741, @Jake-Egan wrote:

Hi, the following assertion is getting tripped on AIX:

Assertion failed: *FpPragmaCurrentValue == SemaObj->FpPragmaStack.DefaultValue && "Expected a default pragma float_control value", file  /scratch/powerllvm/powerllvm_env/aix-ppc64/clang-ppc64-aix/llvm-project/clang/lib/Serialization/ASTReader.cpp, line 8150, void clang::ASTReader::UpdateSema()()

https://lab.llvm.org/buildbot/#/builders/214/builds/8309/steps/6/logs/FAIL__Clang__late-parsed-instantiations_cpp

This crash is observed only on clang-ppc64-aix buildbot. I created an issue for it: https://github.com/llvm/llvm-project/issues/63704 and xfailed the test on this platform: https://reviews.llvm.org/rGd4a5673addd6 .

[thesamesam]

During our regular snapshot testing, we've hit this in Gentoo on 32-bit Linux x86: https://github.com/llvm/llvm-project/issues/63704#issuecomment-1631791518.

clang/include/clang/Sema/Sema.h

FPOptionsOverride CurFPFeatureOverrides() {
     if (!FpPragmaStack.hasValue()) {
       result = FPOptionsOverride();
     } else {
       result = FpPragmaStack.CurrentValue;
     }
     return result;
   }
 
+  void resetFPOptions(FPOptions FPO) {
+    CurFPFeatures = FPO;
+    FpPragmaStack.Stack.clear();
+    FpPragmaStack.CurrentValue = FPO.getChangesFrom(FPOptions(LangOpts));
+  }
+
   // RAII object to push / pop sentinel slots for all MS #pragma stacks.
   // Actions should be performed only if we enter / exit a C++ method body.
   class PragmaStackSentinelRAII {
   public:
     PragmaStackSentinelRAII(Sema &S, StringRef SlotLabel, bool ShouldAct);
     ~PragmaStackSentinelRAII();
 
   private:
MakeDeductionFailureInfo(ASTContext &Context, Sema::TemplateDeductionResult TDK,
                          sema::TemplateDeductionInfo &Info);
 
 /// Contains a late templated function.
 /// Will be parsed at the end of the translation unit, used by Sema & Parser.
 struct LateParsedTemplate {
   CachedTokens Toks;
   /// The template function declaration to be late parsed.
   Decl *D;
+  /// Floating-point options in the point of definition.
+  FPOptions FPO;
 };
 
 template <>
 void Sema::PragmaStack<Sema::AlignPackInfo>::Act(SourceLocation PragmaLocation,
                                                  PragmaMsStackAction Action,
                                                  llvm::StringRef StackSlotLabel,
                                                  AlignPackInfo Value);
 

clang/lib/Parse/ParseTemplate.cpp

for (DeclContext *DC : reverse(DeclContextsToReenter)) {
     CurTemplateDepthTracker.addDepth(
         ReenterTemplateScopes(Scopes, cast<Decl>(DC)));
     Scopes.Enter(Scope::DeclScope);
     // We'll reenter the function context itself below.
     if (DC != FunD)
       Actions.PushDeclContext(Actions.getCurScope(), DC);
   }
 
+  // Parsing should occur with empty FP pragma stack and FP options used in the
+  // point of the template definition.
+  Actions.resetFPOptions(LPT.FPO);
+
   assert(!LPT.Toks.empty() && "Empty body!");
 
   // Append the current token at the end of the new token stream so that it
   // doesn't get lost.
   LPT.Toks.push_back(Tok);
   PP.EnterTokenStream(LPT.Toks, true, /*IsReinject*/true);
 
   // Consume the previously pushed token.

clang/lib/Sema/SemaTemplate.cpp

void Sema::MarkAsLateParsedTemplate(FunctionDecl *FD, Decl *FnD,
   if (!FD)
     return;
 
   auto LPT = std::make_unique<LateParsedTemplate>();
 
   // Take tokens to avoid allocations
   LPT->Toks.swap(Toks);
   LPT->D = FnD;
+  LPT->FPO = getCurFPFeatures();
   LateParsedTemplateMap.insert(std::make_pair(FD, std::move(LPT)));
 
   FD->setLateTemplateParsed(true);
 }
 
 void Sema::UnmarkAsLateParsedTemplate(FunctionDecl *FD) {
   if (!FD)
     return;

clang/test/CodeGen/fp-template.cpp

 float func_01(float x, float y) {
   return templ_01(x, y);
 }
 
 // CHECK-LABEL: define {{.*}} @_Z8templ_01IfET_S0_S0_
 // CHECK-SAME:  (float noundef %{{.*}}, float noundef %{{.*}}) #[[ATTR01:[0-9]+]]{{.*}} {
 // CHECK:       call float @llvm.experimental.constrained.fadd.f32
 
+
+template <typename Ty>
+Ty templ_02(Ty x, Ty y) {
+  return x + y;
+}
+
+#pragma STDC FENV_ROUND FE_UPWARD
+
+template <typename Ty>
+Ty templ_03(Ty x, Ty y) {
+  return x - y;
+}
+
+#pragma STDC FENV_ROUND FE_TONEAREST
+
+float func_02(float x, float y) {
+  return templ_02(x, y);
+}
+
+// CHECK-LABEL: define {{.*}} float @_Z8templ_02IfET_S0_S0_
+// CHECK:       %add = fadd float %0, %1
+
+float func_03(float x, float y) {
+  return templ_03(x, y);
+}
+
+// CHECK-LABEL: define {{.*}} float @_Z8templ_03IfET_S0_S0_
+// CHECK:       call float @llvm.experimental.constrained.fsub.f32({{.*}}, metadata !"round.upward", metadata !"fpexcept.ignore")
+
+
+// This pragma sets non-default rounding mode before delayed parsing occurs. It
+// is used to check that the parsing uses FP options defined by command line
+// options or by pragma before the template definition but not by this pragma.
+#pragma STDC FENV_ROUND FE_TOWARDZERO
+
+
 // CHECK: attributes #[[ATTR01]] = { {{.*}}strictfp