This is an archive of the discontinued LLVM Phabricator instance.

[OpenCL] Defines helper function for OpenCL default address space
ClosedPublic

Authored by Topotuna on Sep 16 2021, 3:35 AM.

Details

Summary

Helper function getDefaultPointeeOpenCLAddrSpace() introduced to
ASTContext class. It returns default OpenCL address space
depending on language version and enabled features. If generic
address space is supported, the helper function returns value
LangAS::opencl_generic. Otherwise, value LangAS::opencl_private
is returned. Code refactoring changes performed in several suitable
places.

Diff Detail

Event Timeline

Topotuna created this revision.Sep 16 2021, 3:35 AM
Topotuna requested review of this revision.Sep 16 2021, 3:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 16 2021, 3:35 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
Topotuna planned changes to this revision.Sep 16 2021, 3:38 AM

Current change is an initial draft. More code refactoring will be performed and change summary provided in near future.

Topotuna updated this revision to Diff 372937.Sep 16 2021, 8:00 AM
Topotuna edited the summary of this revision. (Show Details)
Topotuna added reviewers: olestrohm, Anastasia.

Code refactored inside clang/lib/AST/Expr.cpp. Commit message added.

Anastasia added inline comments.Sep 21 2021, 2:38 AM
clang/include/clang/AST/ASTContext.h
1366

We should probably rename this to getDefaultPointeeOpenCLAddrSpace because this only reflects the default address space for the pointer types.

clang/lib/AST/Expr.cpp
3782–3783

Let's add Ctx.getLangOpts().OpenCL check at the start to make sure this only runs for OpenCL.

Topotuna updated this revision to Diff 373834.Sep 21 2021, 3:14 AM
Topotuna edited the summary of this revision. (Show Details)

Helper function renamed for clarity. Additional check added to if statement to ensure it is only carried out in OpenCL.

Topotuna marked 2 inline comments as done.Sep 21 2021, 3:20 AM

Both comments addressed. Personally, I would shuffle words around to rename helper function as getDefaultOpenCLPointeeAddrSpace or getOpenCLDefaultPointeeAddrSpace. Although I am not sure if there are some assumed naming conventions in Clang. Originally, I was following the name of getDefaultCXXMethodAddrSpace.

Both comments addressed. Personally, I would shuffle words around to rename helper function as getDefaultOpenCLPointeeAddrSpace or getOpenCLDefaultPointeeAddrSpace. Although I am not sure if there are some assumed naming conventions in Clang. Originally, I was following the name of getDefaultCXXMethodAddrSpace.

I agree this naming scheme makes more sense.

Anastasia accepted this revision.Sep 21 2021, 3:42 AM

LGTM! Thanks

Feel free to adjust the naming scheme either in the same commit or separately.

This revision is now accepted and ready to land.Sep 21 2021, 3:42 AM
This revision was landed with ongoing or failed builds.Sep 21 2021, 7:13 AM
This revision was automatically updated to reflect the committed changes.