Skip to content

Commit e643027

Browse files
zahiraamzahiraam
committed
Addressed review comments.
1 parent 56314b7 commit e643027

File tree

4 files changed

+109
-67
lines changed

4 files changed

+109
-67
lines changed

‎clang/lib/CodeGen/CGCall.cpp‎

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1882,30 +1882,40 @@ void CodeGenModule::getDefaultFunctionFPAccuracyAttributes(
18821882
StringRef FPAccuracyVal;
18831883
auto FuncMapIt = getLangOpts().FPAccuracyFuncMap.find(Name.str());
18841884
if (FuncMapIt != getLangOpts().FPAccuracyFuncMap.end()) {
1885-
FPAccuracyVal = llvm::fp::getAccuracyForFPBuiltin(
1886-
ID, FuncType, convertFPAccuracy(FuncMapIt->second));
1885+
if (!getLangOpts().OffloadFP32PrecDiv && Name == "fdiv")
1886+
FPAccuracyVal = "2.5";
1887+
else if (!getLangOpts().OffloadFP32PrecSqrt && Name == "sqrt")
1888+
FPAccuracyVal = "3.0";
1889+
else
1890+
FPAccuracyVal = llvm::fp::getAccuracyForFPBuiltin(
1891+
ID, FuncType, convertFPAccuracy(FuncMapIt->second));
18871892
assert(!FPAccuracyVal.empty() && "A valid accuracy value is expected");
18881893
FuncAttrs.addAttribute("fpbuiltin-max-error", FPAccuracyVal);
18891894
MD = llvm::ConstantAsMetadata::get(llvm::ConstantInt::get(
18901895
Int32Ty, convertFPAccuracyToAspect(FuncMapIt->second)));
18911896
}
18921897
}
18931898
if (FuncAttrs.attrs().size() == 0) {
1894-
StringRef FPAccuracyVal =
1895-
llvm::StringSwitch<StringRef>(Name)
1896-
.Case("sqrt", getLangOpts().OffloadFP32PrecSqrt ? "" : "3.0")
1897-
.Case("fdiv", getLangOpts().OffloadFP32PrecDiv ? "" : "2.5")
1898-
.Default("");
1899-
if (FPAccuracyVal.empty()) {
1900-
if (!getLangOpts().FPAccuracyVal.empty()) {
1899+
if (!getLangOpts().FPAccuracyVal.empty()) {
1900+
StringRef FPAccuracyVal;
1901+
if (!getLangOpts().OffloadFP32PrecDiv && Name == "fdiv")
1902+
FPAccuracyVal = "2.5";
1903+
else if (!getLangOpts().OffloadFP32PrecSqrt && Name == "sqrt")
1904+
FPAccuracyVal = "3.0";
1905+
else
19011906
FPAccuracyVal = llvm::fp::getAccuracyForFPBuiltin(
19021907
ID, FuncType, convertFPAccuracy(getLangOpts().FPAccuracyVal));
1903-
assert(!FPAccuracyVal.empty() && "A valid accuracy value is expected");
1904-
MD = llvm::ConstantAsMetadata::get(llvm::ConstantInt::get(
1905-
Int32Ty, convertFPAccuracyToAspect(getLangOpts().FPAccuracyVal)));
1908+
assert(!FPAccuracyVal.empty() && "A valid accuracy value is expected");
1909+
FuncAttrs.addAttribute("fpbuiltin-max-error", FPAccuracyVal);
1910+
MD = llvm::ConstantAsMetadata::get(llvm::ConstantInt::get(
1911+
Int32Ty, convertFPAccuracyToAspect(getLangOpts().FPAccuracyVal)));
1912+
} else {
1913+
if (!getLangOpts().OffloadFP32PrecDiv && Name == "fdiv") {
1914+
FuncAttrs.addAttribute("fpbuiltin-max-error", "2.5");
1915+
} else if (!getLangOpts().OffloadFP32PrecSqrt && Name == "sqrt") {
1916+
FuncAttrs.addAttribute("fpbuiltin-max-error", "3.0");
19061917
}
19071918
}
1908-
FuncAttrs.addAttribute("fpbuiltin-max-error", FPAccuracyVal);
19091919
}
19101920
}
19111921

‎clang/lib/Driver/ToolChains/Clang.cpp‎

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2959,10 +2959,23 @@ static void EmitAccuracyDiag(const Driver &D, const JobAction &JA,
29592959
}
29602960
}
29612961

2962+
auto SplitFPAccuracyVal = [](StringRef Val) {
2963+
SmallVector<StringRef, 8> ValuesArr;
2964+
SmallVector<StringRef, 8> FuncsArr;
2965+
Val.split(ValuesArr, ":");
2966+
if (ValuesArr.size() > 1) {
2967+
StringRef x = ValuesArr[1];
2968+
x.split(FuncsArr, ",");
2969+
}
2970+
return FuncsArr;
2971+
};
2972+
29622973
static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
29632974
bool OFastEnabled, const ArgList &Args,
29642975
ArgStringList &CmdArgs,
2965-
const JobAction &JA) {
2976+
const JobAction &JA,
2977+
bool &NoOffloadFP32PrecDiv,
2978+
bool &NoOffloadFP32PrecSqrt) {
29662979
// Handle various floating point optimization flags, mapping them to the
29672980
// appropriate LLVM code generation flags. This is complicated by several
29682981
// "umbrella" flags, so we do this by stepping through the flags incrementally
@@ -3007,8 +3020,6 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
30073020
LangOptions::ComplexRangeKind Range = LangOptions::ComplexRangeKind::CX_None;
30083021
std::string ComplexRangeStr = "";
30093022
std::string GccRangeComplexOption = "";
3010-
bool NoOffloadFP32PrecDiv = false;
3011-
bool NoOffloadFP32PrecSqrt = false;
30123023
bool IsDeviceOffloading = JA.isDeviceOffloading(Action::OFK_SYCL);
30133024

30143025
// Lambda to set fast-math options. This is also used by -ffp-model=fast
@@ -3077,7 +3088,6 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
30773088
if (IsDeviceOffloading) {
30783089
if (!FPAccuracy.empty())
30793090
EmitAccuracyDiag(D, JA, FPAccuracy, SPIRVArg);
3080-
30813091
if (SPIRVArg == "-fno-offload-fp32-prec-div")
30823092
NoOffloadFP32PrecDiv = true;
30833093
else if (SPIRVArg == "-fno-offload-fp32-prec-sqrt")
@@ -3090,20 +3100,12 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
30903100
};
30913101

30923102
auto parseFPAccOption = [&](StringRef Val, bool &NoOffloadFlag) {
3093-
SmallVector<StringRef, 8> ValuesArr;
3094-
Val.split(ValuesArr, ":");
3095-
if (ValuesArr.size() == 1)
3096-
NoOffloadFlag = false;
3097-
if (ValuesArr.size() > 1) {
3098-
StringRef x = ValuesArr[1];
3099-
SmallVector<StringRef, 8> FuncsArr;
3100-
x.split(FuncsArr, ",");
3101-
for (const auto &V : FuncsArr) {
3102-
if (V == "fdiv")
3103-
NoOffloadFlag = false;
3104-
else if (V == "sqrt")
3105-
NoOffloadFlag = false;
3106-
}
3103+
SmallVector<StringRef, 8> FuncsArr = SplitFPAccuracyVal(Val);
3104+
for (const auto &V : FuncsArr) {
3105+
if (V == "fdiv")
3106+
NoOffloadFlag = false;
3107+
else if (V == "sqrt")
3108+
NoOffloadFlag = false;
31073109
}
31083110
};
31093111

@@ -5389,6 +5391,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
53895391
Args.hasArg(options::OPT_mkernel, options::OPT_fapple_kext);
53905392
const Driver &D = TC.getDriver();
53915393
ArgStringList CmdArgs;
5394+
bool NoOffloadFP32PrecDiv = false;
5395+
bool NoOffloadFP32PrecSqrt = false;
53925396

53935397
assert(Inputs.size() >= 1 && "Must have at least one input.");
53945398
// CUDA/HIP compilation may have multiple inputs (source file + results of
@@ -6197,7 +6201,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
61976201
options::OPT_fno_optimize_sibling_calls);
61986202

61996203
RenderFloatingPointOptions(TC, D, isOptimizationLevelFast(Args), Args,
6200-
CmdArgs, JA);
6204+
CmdArgs, JA, NoOffloadFP32PrecDiv,
6205+
NoOffloadFP32PrecSqrt);
62016206

62026207
// Render ABI arguments
62036208
switch (TC.getArch()) {
@@ -6671,7 +6676,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
66716676
options::OPT_fno_protect_parens, false))
66726677
CmdArgs.push_back("-fprotect-parens");
66736678

6674-
RenderFloatingPointOptions(TC, D, OFastEnabled, Args, CmdArgs, JA);
6679+
RenderFloatingPointOptions(TC, D, OFastEnabled, Args, CmdArgs, JA,
6680+
NoOffloadFP32PrecDiv, NoOffloadFP32PrecSqrt);
66756681

66766682
if (Arg *A = Args.getLastArg(options::OPT_fextend_args_EQ)) {
66776683
const llvm::Triple::ArchType Arch = TC.getArch();
@@ -6722,8 +6728,18 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
67226728
FpAccuracyAttr += OptStr.str();
67236729
}
67246730
};
6725-
for (StringRef A : Args.getAllArgValues(options::OPT_ffp_accuracy_EQ))
6726-
RenderFPAccuracyOptions(A);
6731+
auto shouldAddFpAccuracyOption = [&](StringRef Val, StringRef Func) {
6732+
SmallVector<StringRef, 8> FuncsArr = SplitFPAccuracyVal(Val);
6733+
for (const auto &V : FuncsArr)
6734+
return (V == Func);
6735+
return false;
6736+
};
6737+
6738+
for (StringRef A : Args.getAllArgValues(options::OPT_ffp_accuracy_EQ)) {
6739+
if (!(NoOffloadFP32PrecDiv && shouldAddFpAccuracyOption(A, "fdiv")) &&
6740+
!(NoOffloadFP32PrecSqrt && shouldAddFpAccuracyOption(A, "sqrt")))
6741+
RenderFPAccuracyOptions(A);
6742+
}
67276743
if (!FpAccuracyAttr.empty())
67286744
CmdArgs.push_back(Args.MakeArgString(FpAccuracyAttr));
67296745

‎clang/test/CodeGenSYCL/offload-fp32-div-sqrt.cpp‎

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,9 @@
6363
// RUN: -ffp-builtin-accuracy=high %s -o - \
6464
// RUN: | FileCheck --check-prefix LOW-PREC-DIV %s
6565

66-
// RUN: %clang_cc1 %{common_opts_spirv32} -fno-offload-fp32-prec-div \
67-
// RUN: -ffp-builtin-accuracy=high:fdiv %s -o - \
68-
// RUN: | FileCheck --check-prefix HIGH-PREC %s
69-
7066
// RUN: %clang_cc1 %{common_opts_spirv32} -ffp-builtin-accuracy=high:fdiv \
7167
// RUN: -fno-offload-fp32-prec-div %s -o - \
72-
// RUN: | FileCheck --check-prefix HIGH-PREC %s
68+
// RUN: | FileCheck --check-prefix ROUNDED-DIV %s
7369

7470
// RUN: %clang_cc1 %{common_opts_spirv32} -fno-offload-fp32-prec-sqrt \
7571
// RUN: -ffp-builtin-accuracy=high %s -o - \
@@ -135,13 +131,9 @@
135131
// RUN: -ffp-builtin-accuracy=high %s -o - \
136132
// RUN: | FileCheck --check-prefix LOW-PREC-DIV %s
137133

138-
// RUN: %clang_cc1 %{common_opts_spirv64} -fno-offload-fp32-prec-div \
139-
// RUN: -ffp-builtin-accuracy=high:fdiv %s -o - \
140-
// RUN: | FileCheck --check-prefix HIGH-PREC %s
141-
142134
// RUN: %clang_cc1 %{common_opts_spirv64} -ffp-builtin-accuracy=high:fdiv \
143135
// RUN: -fno-offload-fp32-prec-div %s -o - \
144-
// RUN: | FileCheck --check-prefix HIGH-PREC %s
136+
// RUN: | FileCheck --check-prefix ROUNDED-DIV %s
145137

146138
// RUN: %clang_cc1 %{common_opts_spirv64} -fno-offload-fp32-prec-sqrt \
147139
// RUN: -ffp-builtin-accuracy=high %s -o - \
@@ -208,13 +200,9 @@
208200
// RUN: -ffp-builtin-accuracy=high %s -o - \
209201
// RUN: | FileCheck --check-prefix LOW-PREC-DIV %s
210202

211-
// RUN: %clang_cc1 %{common_opts_spir} -fno-offload-fp32-prec-div \
212-
// RUN: -ffp-builtin-accuracy=high:fdiv %s -o - \
213-
// RUN: | FileCheck --check-prefix HIGH-PREC %s
214-
215203
// RUN: %clang_cc1 %{common_opts_spir} -ffp-builtin-accuracy=high:fdiv \
216204
// RUN: -fno-offload-fp32-prec-div %s -o - \
217-
// RUN: | FileCheck --check-prefix HIGH-PREC %s
205+
// RUN: | FileCheck --check-prefix ROUNDED-DIV %s
218206

219207
// RUN: %clang_cc1 %{common_opts_spir} -fno-offload-fp32-prec-sqrt \
220208
// RUN: -ffp-builtin-accuracy=high %s -o - \
@@ -280,13 +268,9 @@
280268
// RUN: -ffp-builtin-accuracy=high %s -o - \
281269
// RUN: | FileCheck --check-prefix LOW-PREC-DIV %s
282270

283-
// RUN: %clang_cc1 %{common_opts_spir64} -fno-offload-fp32-prec-div \
284-
// RUN: -ffp-builtin-accuracy=high:fdiv %s -o - \
285-
// RUN: | FileCheck --check-prefix HIGH-PREC %s
286-
287271
// RUN: %clang_cc1 %{common_opts_spir64} -ffp-builtin-accuracy=high:fdiv \
288272
// RUN: -fno-offload-fp32-prec-div %s -o - \
289-
// RUN: | FileCheck --check-prefix HIGH-PREC %s
273+
// RUN: | FileCheck --check-prefix ROUNDED-DIV %s
290274

291275
// RUN: %clang_cc1 %{common_opts_spir64} -fno-offload-fp32-prec-sqrt \
292276
// RUN: -ffp-builtin-accuracy=high %s -o - \

‎clang/test/Driver/offload-fp32-div-sqrt.cpp‎

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,27 +24,43 @@
2424

2525
// RUN: %clang -c -fsycl -ffp-accuracy=high -fno-math-errno \
2626
// RUN: -fno-offload-fp32-prec-div -### %s 2>&1 \
27-
// RUN: | FileCheck %s --check-prefix=WARN-HIGH-DIV
27+
// RUN: | FileCheck %s --check-prefixes=WARN-HIGH-DIV,NO_PREC_DIV_FP_ACC_HIGH
2828

2929
// RUN: %clang -c -fsycl -fno-offload-fp32-prec-div -ffp-accuracy=high \
3030
// RUN: -fno-math-errno -### %s 2>&1 \
31-
// RUN: | FileCheck %s --check-prefix=WARN-HIGH-DIV
31+
// RUN: | FileCheck %s --check-prefixes=WARN-HIGH-DIV,NO_PREC_DIV_FP_ACC_HIGH
32+
33+
// RUN: %clang -c -fsycl -fno-offload-fp32-prec-div -ffp-accuracy=high:fdiv \
34+
// RUN: -fno-math-errno -### %s 2>&1 \
35+
// RUN: | FileCheck %s --check-prefixes=WARN-HIGH-DIV-ONLY,FP_ACC_HIGH_DIV
36+
37+
// RUN: %clang -c -fsycl -ffp-accuracy=high:fdiv \
38+
// RUN: -fno-math-errno -fno-offload-fp32-prec-div -### %s 2>&1 \
39+
// RUN: | FileCheck %s --check-prefixes=WARN-HIGH-DIV-ONLY,NO_PREC_DIV
3240

3341
// RUN: %clang -c -fsycl -fno-offload-fp32-prec-sqrt -ffp-accuracy=high \
3442
// RUN: -fno-math-errno -### %s 2>&1 \
35-
// RUN: | FileCheck %s --check-prefix=WARN-HIGH-SQRT
43+
// RUN: | FileCheck %s --check-prefixes=WARN-HIGH-SQRT,NO_PREC_SQRT_FP_ACC_HIGH
44+
45+
// RUN: %clang -c -fsycl -fno-offload-fp32-prec-sqrt -ffp-accuracy=high:sqrt \
46+
// RUN: -fno-math-errno -### %s 2>&1 \
47+
// RUN: | FileCheck %s --check-prefixes=WARN-HIGH-SQRT-ONLY,FP_ACC_HIGH_SQRT
48+
49+
// RUN: %clang -c -fsycl -ffp-accuracy=high:sqrt \
50+
// RUN: -fno-math-errno -fno-offload-fp32-prec-sqrt -### %s 2>&1 \
51+
// RUN: | FileCheck %s --check-prefixes=WARN-HIGH-SQRT-ONLY,NO_PREC_SQRT
3652

3753
// RUN: %clang -c -fsycl -ffp-accuracy=high -fno-math-errno \
3854
// RUN: -fno-offload-fp32-prec-sqrt -### %s 2>&1 \
39-
// RUN: | FileCheck %s --check-prefix=WARN-HIGH-SQRT
55+
// RUN: | FileCheck %s --check-prefixes=WARN-HIGH-SQRT,NO_PREC_SQRT_FP_ACC_HIGH
4056

4157
// RUN: %clang -c -fsycl -ffp-accuracy=low -fno-math-errno \
4258
// RUN: -fno-offload-fp32-prec-div -### %s 2>&1 \
43-
// RUN: | FileCheck %s --check-prefix=WARN-LOW-DIV
59+
// RUN: | FileCheck %s --check-prefixes=WARN-LOW-DIV,NO_PREC_DIV_FP_ACC_LOW
4460

4561
// RUN: %clang -c -fsycl -ffp-accuracy=low -fno-math-errno \
4662
// RUN: -fno-offload-fp32-prec-sqrt -### %s 2>&1 \
47-
// RUN: | FileCheck %s --check-prefix=WARN-LOW-SQRT
63+
// RUN: | FileCheck %s --check-prefixes=WARN-LOW-SQRT,NO_PREC_SQRT_FP_ACC_LOW
4864

4965
// RUN: %clang -c -fsycl -ffp-model=fast -### %s 2>&1 \
5066
// RUN: | FileCheck --check-prefix=FAST %s
@@ -61,23 +77,39 @@
6177
// RUN: %clang -c -fsycl -ffp-model=fast -foffload-fp32-prec-sqrt -### %s 2>&1 \
6278
// RUN: | FileCheck --check-prefix=NO_PREC_DIV %s
6379

80+
// WARN-HIGH-DIV: floating point accuracy control 'high' conflicts with explicit target precision option '-fno-offload-fp32-prec-div'
81+
82+
// WARN-HIGH-DIV-ONLY: floating point accuracy control 'high:fdiv' conflicts with explicit target precision option '-fno-offload-fp32-prec-div'
83+
84+
// WARN-HIGH-SQRT: floating point accuracy control 'high' conflicts with explicit target precision option '-fno-offload-fp32-prec-sqrt'
85+
86+
// WARN-HIGH-SQRT-ONLY: floating point accuracy control 'high:sqrt' conflicts with explicit target precision option '-fno-offload-fp32-prec-sqrt'
87+
88+
// WARN-LOW-DIV: floating point accuracy control 'low' conflicts with explicit target precision option '-fno-offload-fp32-prec-div'
89+
90+
// WARN-LOW-SQRT: floating point accuracy control 'low' conflicts with explicit target precision option '-fno-offload-fp32-prec-sqrt'
91+
92+
6493
// CHECK: "-triple" "spir64{{.*}}" "-fsycl-is-device"{{.*}}
6594

66-
// CHECK-NOT: "-triple{{.*}}" "-fsycl-is-host"{{.*}} "-foffload-fp32-prec-div" "-foffload-fp32-prec-sqrt"
95+
// CHECK-NOT: "-triple{{.*}}" "-fsycl-is-host"{{.*}} "-foffload-fp32-prec-div" "-foffload-fp32-prec-sqrt"s
6796

6897
// NO_PREC_DIV: "-triple" "spir64{{.*}}"{{.*}} "-fsycl-is-device"{{.*}} "-fno-offload-fp32-prec-div"
6998

7099
// NO_PREC_SQRT: "-triple" "spir64{{.*}}" "-fsycl-is-device"{{.*}} "-fno-offload-fp32-prec-sqrt"
71100

72101
// NO_PREC_DIV_SQRT: "-triple" "spir64{{.*}}" "-fsycl-is-device"{{.*}} "-fno-offload-fp32-prec-div" "-fno-offload-fp32-prec-sqrt"
73102

74-
// WARN-HIGH-DIV: floating point accuracy control 'high' conflicts with explicit target precision option '-fno-offload-fp32-prec-div'
103+
// FAST: "-triple" "spir64{{.*}}"{{.*}} "-fsycl-is-device"{{.*}} "-fno-offload-fp32-prec-div" "-fno-offload-fp32-prec-sqrt"
75104

76-
// WARN-HIGH-SQRT: floating point accuracy control 'high' conflicts with explicit target precision option '-fno-offload-fp32-prec-sqrt'
105+
// FP_ACC_HIGH_DIV: "-triple" "spir64{{.*}}" "-fsycl-is-device"{{.*}} "-ffp-builtin-accuracy=high:fdiv"
77106

78-
// WARN-LOW-DIV: floating point accuracy control 'low' conflicts with explicit target precision option '-fno-offload-fp32-prec-div'
107+
// FP_ACC_HIGH_SQRT: "-triple" "spir64{{.*}}" "-fsycl-is-device"{{.*}} "-ffp-builtin-accuracy=high:sqrt"
79108

80-
// WARN-LOW-SQRT: floating point accuracy control 'low' conflicts with explicit target precision option '-fno-offload-fp32-prec-sqrt'
109+
// NO_PREC_DIV_FP_ACC_HIGH: "-triple" "spir64{{.*}}" "-fsycl-is-device"{{.*}} "-fno-offload-fp32-prec-div" "-ffp-builtin-accuracy=high"
81110

82-
// FAST: "-triple" "spir64{{.*}}"{{.*}} "-fsycl-is-device"{{.*}} "-fno-offload-fp32-prec-div" "-fno-offload-fp32-prec-sqrt"
111+
// NO_PREC_SQRT_FP_ACC_HIGH: "-triple" "spir64{{.*}}" "-fsycl-is-device"{{.*}} "-fno-offload-fp32-prec-sqrt" "-ffp-builtin-accuracy=high"
112+
113+
// NO_PREC_DIV_FP_ACC_LOW: "-triple" "spir64{{.*}}" "-fsycl-is-device"{{.*}} "-fno-offload-fp32-prec-div" "-ffp-builtin-accuracy=low"
83114

115+
// NO_PREC_SQRT_FP_ACC_LOW: "-triple" "spir64{{.*}}" "-fsycl-is-device"{{.*}} "-fno-offload-fp32-prec-sqrt" "-ffp-builtin-accuracy=low"

0 commit comments

Comments
 (0)