This is an archive of the discontinued LLVM Phabricator instance.

[OpenCL] Use DW_LANG_OpenCL language tag for OpenCL C
ClosedPublic

Authored by stuart on Jun 11 2021, 7:21 AM.

Details

Summary

Note regarding C++ for OpenCL:

When compiling C++ for OpenCL, DW_LANG_C_plus_plus* is emitted.

There is no DWARF language code defined for C++ for OpenCL as of yet, but DWARF issue 210514.1 has been raised to request one.

In the mean time, continuing to emit DW_LANG_C_plus_plus* for C++ for OpenCL allows the potential to distinguish between C++ for OpenCL and OpenCL C in !DICompileUnit nodes, whereas using DW_LANG_OpenCL for C++ for OpenCL would prevent this.

This change therefore leaves C++ for OpenCL as-is.

Diff Detail

Unit TestsFailed

Event Timeline

stuart created this revision.Jun 11 2021, 7:21 AM
stuart requested review of this revision.Jun 11 2021, 7:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 11 2021, 7:21 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Note: there is currently no DWARF language code defined for C++ for OpenCL, so we must use DW_LANG_C_plus_plus* if we wish to be able to determine whether output has been generated from C++ for OpenCL source or from OpenCL C source. I have raised DWARF issue 210514.1 to add a dedicated C++ for OpenCL code in the next version of DWARF, but for now I believe that it is best to use DW_LANG_OpenCL for OpenCL C only, and not for C++ for OpenCL.

I could perhaps add a note regarding this to the commit message but am concerned about overcomplicating the message.

stuart updated this revision to Diff 351454.Jun 11 2021, 8:10 AM

Add missing trailing commas to CHECK lines of FileCheck test.

SouraVX added a comment.EditedJun 13 2021, 9:57 PM

Note: there is currently no DWARF language code defined for C++ for OpenCL, so we must use DW_LANG_C_plus_plus* if we wish to be able to determine whether output has been generated from C++ for OpenCL source or from OpenCL C source. I have raised DWARF issue 210514.1 to add a dedicated C++ for OpenCL code in the next version of DWARF, but for now I believe that it is best to use DW_LANG_OpenCL for OpenCL C only, and not for C++ for OpenCL.

I could perhaps add a note regarding this to the commit message but am concerned about overcomplicating the message.

NIT: It would be nice to have this as part of the commit summary/message itself. Min. at least the DWARF issue part.

shchenz added inline comments.Jun 14 2021, 6:16 PM
clang/lib/CodeGen/CGDebugInfo.cpp
579

I think for non-strict DWARF mode, we still can generate DW_LANG_OpenCL for the DWARF version lower than 5? Seems we also need to fix above DW_LANG_C_plus_plus_14 and DW_LANG_C_plus_plus_11

stuart added inline comments.Jun 15 2021, 2:59 AM
clang/lib/CodeGen/CGDebugInfo.cpp
579

If we have a -gstrict-dwarf option for this, then it would seem better to add DW_LANG_C_plus_plus_17 and DW_LANG_C_plus_plus_20 definitions and generate those for the !CGM.getCodeGenOpts().DebugStrictDwarf case. C++ for OpenCL would then use one of the more recent language tag values for the time being (without any special logic).

shchenz added inline comments.Jun 15 2021, 4:30 AM
clang/lib/CodeGen/CGDebugInfo.cpp
579

I added a patch https://reviews.llvm.org/D104291 for DW_LANG_C_plus_plus_14 and DW_LANG_C_plus_plus_11.
I think DW_LANG_OpenCL should be in the same situation?

stuart marked an inline comment as done.Jun 15 2021, 7:26 AM
stuart added inline comments.
clang/lib/CodeGen/CGDebugInfo.cpp
579

Thanks. I will wait until D104291 is accepted, and then update this change correspondingly. Yes, I believe DW_LANG_OpenCL is in the same situation.

Looking more closely, I notice that DW_LANG_RenderScript was also introduced in DWARF 5, and we don't use it (at all) in Clang, but instead use the vendor-specific DW_LANG_GOOGLE_RenderScript value. It should probably fall back to the vendor-specific value if the DWARF 5 value is not available, instead.

stuart marked an inline comment as done.Jun 15 2021, 7:26 AM
stuart planned changes to this revision.Jun 15 2021, 7:45 AM

Changes will be required to align this with D104291.

stuart updated this revision to Diff 353988.Jun 23 2021, 8:15 AM

Added handling of -gstrict-dwarf and updated tests accordingly.

stuart edited the summary of this revision. (Show Details)Jun 23 2021, 8:16 AM
stuart edited the summary of this revision. (Show Details)
shchenz accepted this revision.Jun 23 2021, 6:21 PM

LGTM. Thanks.

This revision is now accepted and ready to land.Jun 23 2021, 6:21 PM
Anastasia accepted this revision.Jun 24 2021, 6:44 AM

LGTM! Thanks!

This revision was automatically updated to reflect the committed changes.

On reflection, I don't think it makes sense to make use of DW_LANG_C_plus_plus_17 or DW_LANG_C_plus_plus_20 in Clang just yet, as these are generally not supported by other tooling. I am a bit confused by DWARF publishing these tags ahead of time, yet tooling having not been updated to recognize them. Regardless, it is clear that support in debuggers needs to come first, which is something that I am not planning to add.