Skip to content

Fix crash when using -ffp-accuracy and std:: trigonometric functions. #11606

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Oct 30, 2023
Merged

Fix crash when using -ffp-accuracy and std:: trigonometric functions. #11606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
51cbe1b
Fix crash
zahiraam Oct 11, 2023
adf858f
Added LIT test
zahiraam Oct 19, 2023
009f0ce
Fix format
zahiraam Oct 19, 2023
dfd7c10
Fix format again
zahiraam Oct 19, 2023
0d1b219
Updated LIT test
zahiraam Oct 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions clang/lib/CodeGen/CGCall.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5684,10 +5684,11 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
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);
if (CI)
return RValue::get(CI);
if (FD) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When do we get a null FD here? Can we assert that condition instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We get the null when the entire sycl.hpp is included. And to use std::sin we need to include the entire sycl.hpp.

Copy link
Contributor

@premanandrao premanandrao Oct 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean what property of this entity is it that causes FD to be NULL? Can we assert (FD || that condition) instead of removing the non-NULL FD check that we had before?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FD is null when TargetDecl is not a FunctionDecl. Not really sure what "that condition" should be?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chatted with @zahiraam offline; she believes that all the null FD cases are understood and handled here, so no assertion is necessary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @premanandrao for the updates about NULL cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just an additional comment about the assert.
For any TargetDecl, the control flow passes through this code. In the case we are using the fp-accuracy we need to check that the TargetDecl is a FunctionDecl and only then we can generate a call to the intrinsic. For any other kind of TargetDecl we can just create the call (in this particular case, the TargetDecl was a CXXConstructorDecl).

CI = EmitFPBuiltinIndirectCall(IRFuncTy, IRCallArgs, CalleePtr, FD);
if (CI)
return RValue::get(CI);
}
}
CI = Builder.CreateCall(IRFuncTy, CalleePtr, IRCallArgs, BundleList);
} else {
Expand Down
18 changes: 18 additions & 0 deletions sycl/test/basic_tests/fp-accuracy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// RUN: %clangxx -%fsycl-host-only -c -ffp-accuracy=high -faltmathlib=SVMLAltMathLibrary -fno-math-errno %s

#include <sycl/sycl.hpp>
using namespace sycl;

int main() {
queue deviceQueue;
double Value = 5.;

deviceQueue.submit([&](handler &cgh) {
cgh.single_task<class Kernel0>([=]() { double res = std::sin(Value); });
});

deviceQueue.submit([&](handler &cgh) {
cgh.single_task<class Kernel1>([=]() { double res = sycl::sin(Value); });
});
return 0;
}