-
Notifications
You must be signed in to change notification settings - Fork 787
[SYCL][FE][Driver] Implement floating point accuracy control #8280
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 12 commits
79792b3
c3029ee
fa51800
b584e3d
ec2dfbc
be478d8
4c23010
db8c415
2bcd2a8
bdf5dbe
d796b5f
6c005d8
b93d9ec
17eb30f
4ffd86f
bd458b9
80c131a
faae1a0
bbabbb5
ef29dfc
8aabc41
6681fc2
20fd09b
300d2fe
1e148b3
4f6089a
5a61b28
f02d393
061ca9f
edb89ce
ec3be13
9cce8b7
2ea8d5a
7abea95
773f1ed
68c9a70
a7c7546
bffc1fd
7900b87
a71e5f2
a05458f
02831e8
680bd47
c3f2ca2
10715d3
a3803db
98ed36d
cdfec2c
831aa36
a0adb9e
d3d470e
e08362e
6d5890d
7d3c976
9c86522
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,6 +61,10 @@ def err_drv_no_cuda_libdevice : Error< | |
"via '--cuda-path', or pass '-nocudalib' to build without linking with " | ||
"libdevice">; | ||
|
||
def warn_function_fp_accuray_already_set : Warning <"FP accuracy value of '%0' has already " | ||
"been assigned to function '%1'">; | ||
def warn_all_fp_accuray_already_set : Warning <"FP accuracy value of '%0' has already " | ||
zahiraam marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"been assigned to all functions in the program">; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should there be a way to disable these diagnostics? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why would we want to disable them? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is an informative diagnostic, but typically warnings have a way to be disabled. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can create the warning in a new group: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, that's what I mean. Maybe: "fp-accuracy-value" would be sufficient here for the name. |
||
def err_drv_no_rocm_device_lib : Error< | ||
"cannot find ROCm device library%select{| for %1|for ABI version %1}0; provide its path via " | ||
"'--rocm-path' or '--rocm-device-lib-path', or pass '-nogpulib' to build " | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,4 +26,5 @@ OPTION(AllowReciprocal, bool, 1, NoSignedZero) | |
OPTION(AllowApproxFunc, bool, 1, AllowReciprocal) | ||
OPTION(FPEvalMethod, LangOptions::FPEvalMethodKind, 2, AllowApproxFunc) | ||
OPTION(Float16ExcessPrecision, LangOptions::ExcessPrecisionKind, 2, FPEvalMethod) | ||
OPTION(FPAccuracy, LangOptions::FPAccuracyKind, 2, Float16ExcessPrecision) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This requires 3 bits to store all the values of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed it to be 3 bits, but not sure what testing I should be doing to catch the error? The LIT test in the Driver folder is exercising all values of FPAccuracyKind and was passing with the 2 bits width. |
||
#undef OPTION |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ | |
#include "clang/Basic/TargetCXXABI.h" | ||
#include "clang/Basic/Visibility.h" | ||
#include "llvm/ADT/FloatingPointMode.h" | ||
#include "llvm/ADT/MapVector.h" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see a map-vector being used? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Left over from previous version. Removed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this still here for the std::map usage below? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The include is still here? |
||
#include "llvm/ADT/StringRef.h" | ||
#include "llvm/TargetParser/Triple.h" | ||
#include <optional> | ||
|
@@ -303,6 +304,15 @@ class LangOptions : public LangOptionsBase { | |
|
||
enum ExcessPrecisionKind { FPP_Standard, FPP_Fast, FPP_None }; | ||
|
||
enum FPAccuracyKind { | ||
FPA_Default, | ||
FPA_High, | ||
FPA_Medium, | ||
FPA_Low, | ||
FPA_Sycl, | ||
FPA_Cuda | ||
zahiraam marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}; | ||
|
||
/// Possible exception handling behavior. | ||
enum class ExceptionHandlingKind { None, SjLj, WinEH, DwarfCFI, Wasm }; | ||
|
||
|
@@ -386,6 +396,13 @@ class LangOptions : public LangOptionsBase { | |
IncompleteOnly = 3, | ||
}; | ||
|
||
using FPAccuracyMapTy = | ||
llvm::MapVector<std::string, std::string, llvm::StringMap<unsigned>>; | ||
FPAccuracyMapTy FPAccuracyMap; | ||
using FPAccuracyFuncMapTy = | ||
llvm::MapVector<std::string, FPAccuracyMapTy, llvm::StringMap<unsigned>>; | ||
FPAccuracyFuncMapTy FPAccuracyFuncMap; | ||
|
||
public: | ||
/// The used language standard. | ||
LangStandard::Kind LangStd; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3323,6 +3323,21 @@ void CompilerInvocation::GenerateLangArgs(const LangOptions &Opts, | |
#include "clang/Driver/Options.inc" | ||
#undef LANG_OPTION_WITH_MARSHALLING | ||
|
||
for (const auto &M : Opts.FPAccuracyMap) { | ||
SmallString<128> S; | ||
S += M.second; | ||
GenerateArg(Args, OPT_ffp_accuracy_attr_EQ, S, SA); | ||
} | ||
zahiraam marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for (const auto &F : Opts.FPAccuracyFuncMap) { | ||
for (const auto &C : F.second) { | ||
SmallString<128> S; | ||
S += C.second; | ||
S += ':'; | ||
S += F.first; | ||
GenerateArg(Args, OPT_ffp_accuracy_attr_EQ, S, SA); | ||
} | ||
} | ||
zahiraam marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// The '-fcf-protection=' option is generated by CodeGenOpts generator. | ||
|
||
if (Opts.ObjC) { | ||
|
@@ -3565,6 +3580,65 @@ void CompilerInvocation::GenerateLangArgs(const LangOptions &Opts, | |
GenerateArg(Args, OPT_fno_gpu_rdc, SA); | ||
} | ||
|
||
void CompilerInvocation::ParseFpAccuracyArgs(LangOptions &Opts, ArgList &Args, | ||
DiagnosticsEngine &Diags) { | ||
for (StringRef Values : Args.getAllArgValues(OPT_ffp_accuracy_attr_EQ)) { | ||
SmallVector<StringRef, 8> ValuesArr; | ||
Values.split(ValuesArr, ' '); | ||
for (const auto &Val : ValuesArr) { | ||
SmallVector<StringRef, 3> ValElement; | ||
Val.split(ValElement, ':'); | ||
// The option is of the form -ffp-accuracy=value. | ||
if (ValElement.size() == 1) { | ||
StringRef FPAccuracy = ValElement[0]; | ||
if (!(FPAccuracy.equals("default") || FPAccuracy.equals("high") || | ||
FPAccuracy.equals("low") || FPAccuracy.equals("medium") || | ||
FPAccuracy.equals("sycl") || FPAccuracy.equals("cuda"))) | ||
Diags.Report(diag::err_drv_unsupported_option_argument) | ||
<< "ffp-accuracy" << FPAccuracy; | ||
std::pair<LangOptions::FPAccuracyMapTy::iterator, bool> Result = | ||
Opts.FPAccuracyMap.insert({"fp-accuracy", FPAccuracy.str()}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand what FPAccuracyMap is mapping. What is the difference between this and FPAccuracyFuncMap? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FPAccuracyMap creates a map between fp-accuracy and the value of accuracy given of the command line. FPAccuracyFuncMap will create a map between the functions on the command line and an FPAccuracyMap. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So is there ever more than one entry in FPAccuracyMap? |
||
if (!Result.second) { | ||
Diags.Report(diag::warn_all_fp_accuray_already_set) | ||
<< Result.first->second; | ||
} | ||
} | ||
// The option is of the form -ffp-accuracy=value:[f1, ... fn]. | ||
if (ValElement.size() == 2) { | ||
SmallVector<StringRef, 30> FuncList; | ||
ValElement[1].split(FuncList, ','); | ||
for (StringRef FuncName : FuncList) { | ||
if (FuncName.front() == '[') | ||
FuncName = FuncName.drop_front(1); | ||
if (FuncName.back() == ']') | ||
FuncName = FuncName.drop_back(1); | ||
auto FuncMapIt = Opts.FPAccuracyFuncMap.find(FuncName.str()); | ||
if (FuncMapIt != Opts.FPAccuracyFuncMap.end()) { | ||
// The math function has already been assigned an fp accuracy. | ||
std::pair<LangOptions::FPAccuracyMapTy::iterator, bool> Result = | ||
FuncMapIt->second.insert({"fp-accuracy", ValElement[0].str()}); | ||
if (!Result.second) { | ||
Diags.Report(diag::warn_function_fp_accuray_already_set) | ||
<< Result.first->second << FuncName.str(); | ||
} | ||
} else { | ||
LangOptions::FPAccuracyMapTy FPAccMap; | ||
StringRef FPAccuracy = ValElement[0]; | ||
if (!(FPAccuracy.equals("default") || FPAccuracy.equals("high") || | ||
FPAccuracy.equals("low") || FPAccuracy.equals("medium") || | ||
FPAccuracy.equals("sycl") || FPAccuracy.equals("cuda"))) | ||
Diags.Report(diag::err_drv_unsupported_option_argument) | ||
<< "ffp-accuracy" << FPAccuracy; | ||
FPAccMap.insert({"fp-accuracy", FPAccuracy.str()}); | ||
Opts.FPAccuracyFuncMap.insert( | ||
{FuncName.str(), std::move(FPAccMap)}); | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args, | ||
InputKind IK, const llvm::Triple &T, | ||
std::vector<std::string> &Includes, | ||
|
@@ -3721,6 +3795,8 @@ bool CompilerInvocation::ParseLangArgs(LangOptions &Opts, ArgList &Args, | |
#include "clang/Driver/Options.inc" | ||
#undef LANG_OPTION_WITH_MARSHALLING | ||
|
||
ParseFpAccuracyArgs(Opts, Args, Diags); | ||
|
||
if (const Arg *A = Args.getLastArg(OPT_fcf_protection_EQ)) { | ||
StringRef Name = A->getValue(); | ||
if (Name == "full" || Name == "branch") { | ||
|
Uh oh!
There was an error while loading. Please reload this page.