[SYCL][FE][Driver] Re-enable fp-accuracy tests (#9978)

Both issues were due to the DoRoundTrip value (1 in debug compile and 0
in release compile).
Diagnostics were generated in a function dependent on the value of
DoRoundTrip and that had the effect of generating no diagnostic in
release mode, but diagnostics were fired in debug more in the second run
on the function RoundTrip.
Same for the setting of the codegen’s FPAccuracy option.
Changes:
1- Removed un-necessary FPaccuracy codegen option; used instead the
values of FpAccuracyVal and FPAccuracyFuncMap.
2- Changed place to the diagnostics. 
@AaronBallman what do you think of the placement of the diagnostics? Is
this acceptable for potential upstream?

Fixes: #9934

Author: zahiraam

clang/include/clang/Basic/CodeGenOptions.def

@@ -373,10 +373,6 @@ CODEGENOPT(VirtualFunctionElimination, 1, 0) ///< Whether to apply the dead
                                              /// virtual function elimination
                                              /// optimization.
 
-/// Whether accuracy levels for math library functions are requested by the
-/// user. These accuracy levels will then be expressed in terms of ULPs.
-CODEGENOPT(FPAccuracy, 1, 0)
-
 /// Whether to use public LTO visibility for entities in std and stdext
 /// namespaces. This is enabled by clang-cl's /MT and /MTd flags.
 CODEGENOPT(LTOVisibilityPublicStd, 1, 0)

clang/lib/CodeGen/CGBuiltin.cpp

@@ -537,33 +537,45 @@ static bool hasAccuracyRequirement(CodeGenFunction &CGF, StringRef Name) {
   return FuncMapIt != CGF.getLangOpts().FPAccuracyFuncMap.end();
 }
 
-// Emit a simple mangled intrinsic that has 1 argument and a return type
-// matching the argument type. Depending on mode, this may be a constrained
-// or an fpbuiltin floating-point intrinsic.
-static Value *emitUnaryMaybeConstrainedFPBuiltin(
-    CodeGenFunction &CGF, const CallExpr *E, unsigned IntrinsicID,
-    unsigned ConstrainedIntrinsicID,
-    unsigned FPAccuracyIntrinsicID = Intrinsic::not_intrinsic) {
-  llvm::Value *Src0 = CGF.EmitScalarExpr(E->getArg(0));
+static Function *emitMaybeIntrinsic(CodeGenFunction &CGF, const CallExpr *E,
+                                    unsigned FPAccuracyIntrinsicID,
+                                    unsigned IntrinsicID, llvm::Value *Src0,
+                                    StringRef &Name) {
+  Function *Func = nullptr;
   if (FPAccuracyIntrinsicID != Intrinsic::not_intrinsic) {
-    if (CGF.CGM.getCodeGenOpts().FPAccuracy) {
+    if (!CGF.getLangOpts().FPAccuracyVal.empty() ||
+        !CGF.getLangOpts().FPAccuracyFuncMap.empty()) {
       if (CGF.getLangOpts().MathErrno) {
         DiagnosticsEngine &Diags = CGF.CGM.getDiags();
         Diags.Report(E->getBeginLoc(), diag::err_drv_incompatible_options)
             << "-ffp-accuracy"
             << "-fmath-errno";
       } else {
-        StringRef Name =
+        Name =
             CGF.CGM.getContext().BuiltinInfo.getName(CGF.getCurrentBuiltinID());
         // Use fpbuiltin intrinsic only when needed.
-        Function *Func =
-            getIntrinsic(CGF, Src0, FPAccuracyIntrinsicID, IntrinsicID,
-                         hasAccuracyRequirement(CGF, Name));
-        return CreateBuiltinCallWithAttr(CGF, Name, Func, {Src0},
-                                         FPAccuracyIntrinsicID);
+        Func = getIntrinsic(CGF, Src0, FPAccuracyIntrinsicID, IntrinsicID,
+                            hasAccuracyRequirement(CGF, Name));
       }
     }
   }
+  return Func;
+}
+
+// Emit a simple mangled intrinsic that has 1 argument and a return type
+// matching the argument type. Depending on mode, this may be a constrained
+// or an fpbuiltin floating-point intrinsic.
+static Value *emitUnaryMaybeConstrainedFPBuiltin(
+    CodeGenFunction &CGF, const CallExpr *E, unsigned IntrinsicID,
+    unsigned ConstrainedIntrinsicID,
+    unsigned FPAccuracyIntrinsicID = Intrinsic::not_intrinsic) {
+  llvm::Value *Src0 = CGF.EmitScalarExpr(E->getArg(0));
+  StringRef Name;
+  Function *Func = emitMaybeIntrinsic(CGF, E, FPAccuracyIntrinsicID,
+                                      IntrinsicID, Src0, Name);
+  if (Func)
+    return CreateBuiltinCallWithAttr(CGF, Name, Func, {Src0},
+                                     FPAccuracyIntrinsicID);
   if (CGF.Builder.getIsFPConstrained()) {
     CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, E);
     Function *F = CGF.CGM.getIntrinsic(ConstrainedIntrinsicID, Src0->getType());
@@ -582,15 +594,12 @@ static Value *emitBinaryMaybeConstrainedFPBuiltin(
     unsigned FPAccuracyIntrinsicID = Intrinsic::not_intrinsic) {
   llvm::Value *Src0 = CGF.EmitScalarExpr(E->getArg(0));
   llvm::Value *Src1 = CGF.EmitScalarExpr(E->getArg(1));
-  if (CGF.CGM.getCodeGenOpts().FPAccuracy) {
-    StringRef Name =
-        CGF.CGM.getContext().BuiltinInfo.getName(CGF.getCurrentBuiltinID());
-    // Use fpbuiltin intrinsic only when needed.
-    Function *Func = getIntrinsic(CGF, Src0, FPAccuracyIntrinsicID, IntrinsicID,
-                                  hasAccuracyRequirement(CGF, Name));
+  StringRef Name;
+  Function *Func = emitMaybeIntrinsic(CGF, E, FPAccuracyIntrinsicID,
+                                      IntrinsicID, Src0, Name);
+  if (Func)
     return CreateBuiltinCallWithAttr(CGF, Name, Func, {Src0, Src1},
                                      FPAccuracyIntrinsicID);
-  }
   if (CGF.Builder.getIsFPConstrained()) {
     CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, E);
     Function *F = CGF.CGM.getIntrinsic(ConstrainedIntrinsicID, Src0->getType());

clang/lib/CodeGen/CGCall.cpp

@@ -5620,7 +5620,8 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
   // Emit the actual call/invoke instruction.
   llvm::CallBase *CI;
   if (!InvokeDest) {
-    if (CGM.getCodeGenOpts().FPAccuracy) {
+    if (!getLangOpts().FPAccuracyFuncMap.empty() ||
+        !getLangOpts().FPAccuracyVal.empty()) {
       const auto *FD = dyn_cast_if_present<FunctionDecl>(TargetDecl);
       assert(FD && "expecting a function");
       CI = EmitFPBuiltinIndirectCall(IRFuncTy, IRCallArgs, CalleePtr, FD);

clang/lib/Frontend/CompilerInvocation.cpp

@@ -504,6 +504,12 @@ static bool FixupInvocation(CompilerInvocation &Invocation,
     LangOpts.NewAlignOverride = 0;
   }
 
+  // Diagnose FPAccuracy option validity.
+  if (!LangOpts.FPAccuracyVal.empty())
+    for (const auto &F : LangOpts.FPAccuracyFuncMap)
+      Diags.Report(diag::warn_function_fp_accuracy_already_set)
+          << F.second << F.first;
+
   // Prevent the user from specifying both -fsycl-is-device and -fsycl-is-host.
   if (LangOpts.SYCLIsDevice && LangOpts.SYCLIsHost)
     Diags.Report(diag::err_drv_argument_not_allowed_with) << "-fsycl-is-device"
@@ -2034,9 +2040,6 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
     }
   }
 
-  if (Args.getLastArg(options::OPT_ffp_builtin_accuracy_EQ))
-    Opts.FPAccuracy = 1;
-
   if (auto *arg =
           Args.getLastArg(options::OPT_fdiagnostics_misexpect_tolerance_EQ)) {
     auto ResultOrErr = parseToleranceOption(arg->getValue());
@@ -3620,22 +3623,12 @@ void CompilerInvocation::ParseFpAccuracyArgs(LangOptions &Opts, ArgList &Args,
             if (FuncName.back() == ']')
               FuncName = FuncName.drop_back(1);
             auto FuncMap = Opts.FPAccuracyFuncMap.find(FuncName.str());
-            if (FuncMap != Opts.FPAccuracyFuncMap.end()) {
-              if (!FuncMap->second.empty()) {
-                Diags.Report(diag::warn_function_fp_accuracy_already_set)
-                    << FuncMap->second << FuncName.str();
-              }
-            } else {
-              checkFPAccuracyIsValid(ValElement[0], Diags);
-              if (!Opts.FPAccuracyVal.empty())
-                Diags.Report(diag::warn_function_fp_accuracy_already_set)
-                    << Opts.FPAccuracyVal << FuncName.str();
-              // No need to fill the map if the FPaccuracy is 'default'.
-              // The default builtin will be generated.
-              if (!ValElement[0].equals("default"))
-                Opts.FPAccuracyFuncMap.insert(
-                    {FuncName.str(), ValElement[0].str()});
-            }
+            checkFPAccuracyIsValid(ValElement[0], Diags);
+            // No need to fill the map if the FPaccuracy is 'default'.
+            // The default builtin will be generated.
+            if (!ValElement[0].equals("default"))
+              Opts.FPAccuracyFuncMap.insert(
+                  {FuncName.str(), ValElement[0].str()});
           }
         }
       }

clang/test/CodeGen/fp-accuracy.c

@@ -30,9 +30,6 @@
 // RUN: -Wno-return-type -Wno-implicit-function-declaration -emit-llvm -o - %s \
 // RUN: | FileCheck --check-prefixes=CHECK-DEFAULT %s
 
-// Disabled due to https://github.com/intel/llvm/issues/9934
-// UNSUPPORTED: system-linux
-
 #ifdef SPIR
 // This is a declaration when compiling with -fsycl to avoid
 // the compilation error "function with no prototype cannot use

clang/test/Driver/fp-accuracy.c

@@ -36,7 +36,7 @@
 // RUN: not %clang -Xclang -verify -fno-math-errno -ffp-accuracy=high=[sin] %s 2>& 1 \
 // RUN: | FileCheck %s --check-prefixes=ERR-2
 
-// RUN: not %clang -Xclang -verify -fno-math-errno -ffp-accuracy=low:[sin,cos] \
+// RUN: not %clang -fno-math-errno -ffp-accuracy=low:[sin,cos] \
 // RUN: -ffp-accuracy=high %s 2>&1  \
 // RUN: | FileCheck %s --check-prefix=WARN
 
@@ -48,9 +48,6 @@
 // RUN: -fmath-errno %s 2>&1  \
 // RUN: | FileCheck %s --check-prefixes=ERR-3
 
-// Disabled due to https://github.com/intel/llvm/issues/9934
-// UNSUPPORTED: system-linux
-
 // HIGH: "-ffp-builtin-accuracy=high"
 // LOW: "-ffp-builtin-accuracy=low"
 // MEDIUM: "-ffp-builtin-accuracy=medium"
@@ -61,6 +58,7 @@
 // ERR: (frontend): unsupported argument 'foo' to option '-ffp-accuracy'
 // ERR-1: (frontend): unsupported argument 'foo' to option '-ffp-accuracy'
 // ERR-2: (frontend): unsupported argument 'high=[sin]' to option '-ffp-accuracy'
-// WARN: (frontend): floating point accuracy value of 'high' has already been assigned to function 'cos'
-// WARN: (frontend): floating point accuracy value of 'high' has already been assigned to function 'sin'
+// WARN: floating point accuracy value of 'low' has already been assigned to function 'cos'
+// WARN: floating point accuracy value of 'low' has already been assigned to function 'sin'
+
 // ERR-3: (frontend): floating point accuracy requirements cannot be guaranteed when '-fmath-errno' is enabled; use '-fno-math-errno' to enable floating point accuracy control