Page MenuHomePhabricator

[CUDA] Work around compatibility issue with libstdc++ 11.1.0
ClosedPublic

Authored by tra on May 21 2021, 11:00 AM.

Details

Summary

libstdc++ 11.1.0 redeclares failed_assertion multiple times and that results in the
function declared with conflicting set of attributes when we include <complex>
with
host__ device attributes force-applied to all functions.

In order to work around the issue, we rename __failed_assertion within the
region with forced attributes.

See https://bugs.llvm.org/show_bug.cgi?id=50383 for the details.

Diff Detail

Event Timeline

tra created this revision.May 21 2021, 11:00 AM
tra requested review of this revision.May 21 2021, 11:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 21 2021, 11:00 AM
jlebar accepted this revision.May 21 2021, 11:13 AM
This revision is now accepted and ready to land.May 21 2021, 11:13 AM

Sadly don't have stdibc++11 available locally as well.. so can't verify, but looks good to me except for the push and pop macros.

clang/lib/Headers/cuda_wrappers/complex
77

Not sure I understand what the push of __failed_assert is for..? can't find any reference to __failed_assert anywhere... did you mean to write __failed_assertion?

tra added inline comments.May 21 2021, 11:58 AM
clang/lib/Headers/cuda_wrappers/complex
77

It's a precaution -- we have no control over what may or may not be defined outside of this header.

tra updated this revision to Diff 347092.May 21 2021, 12:01 PM
tra edited the summary of this revision. (Show Details)

Fixed typo in push/pop macro name.

clang/lib/Headers/cuda_wrappers/complex
77

That, and I've made a typo. Fixed now.

fodinabor accepted this revision.May 21 2021, 12:22 PM

LGTM :)

jwakely requested changes to this revision.EditedMay 21 2021, 2:02 PM

You can't use __GLIBCXX__ this way. It will be different for different snapshots from the gcc-11 branch. Some distros are already shipping gcc-11 snapshots with later dates.

I would just check RELEASE == 11. If __failed_assertion is present, you'll rename it. If it's not present, nothing gets renamed but it works anyway.

Or you could pick a date next week and assume I'll have removed the __failed_assertion declarations by then (it should happen on Monday or Tuesday).

This revision now requires changes to proceed.May 21 2021, 2:02 PM
tra updated this revision to Diff 347133.May 21 2021, 3:01 PM

Check only _GLIBCXX_RELEASE

tra added a comment.May 21 2021, 3:07 PM

You can't use __GLIBCXX__ this way. It will be different for different snapshots from the gcc-11 branch. Some distros are already shipping gcc-11 snapshots with later dates.

I would just check RELEASE == 11. If __failed_assertion is present, you'll rename it. If it's not present, nothing gets renamed but it works anyway.

Good point. I've updated the patch.

yaxunl added inline comments.May 21 2021, 3:18 PM
clang/lib/Headers/cuda_wrappers/complex
79

May I ask where is __cuda_failed_assertion defined? Thanks.

tra added inline comments.May 21 2021, 4:26 PM
clang/lib/Headers/cuda_wrappers/complex
79

The function is not defined anywhere.

From https://bugs.llvm.org/show_bug.cgi?id=50383#c14

Its sole purpose is to be a non-constexpr function that can be called during constant
evaluation to cause a compilation error. It can't be called at runtime, because
compilation will fail if control flow ever results in calling it. So no definition is needed.

jwakely accepted this revision.May 22 2021, 3:00 AM

LGTM

This revision is now accepted and ready to land.May 22 2021, 3:00 AM
This revision was automatically updated to reflect the committed changes.