This is an archive of the discontinued LLVM Phabricator instance.

[CUDA][HIP] Fix hostness check with -fopenmp
ClosedPublic

Authored by yaxunl on Mar 15 2022, 7:56 PM.

Details

Summary

CUDA/HIP determines whether a function can be called based on
the device/host attributes of callee and caller. Clang assumes the
caller is CurContext. This is correct in most cases, however, it is
not correct in OpenMP parallel region when CUDA/HIP program
is compiled with -fopenmp. This causes incorrect overloading
resolution and missed diagnostics.

To get the correct caller, clang needs to chase the parent chain
of DeclContext starting from CurContext until a function decl
or a lambda decl is reached. Sema API is adapted to achieve that
and used to determine the caller in hostness check.

Diff Detail

Event Timeline

yaxunl created this revision.Mar 15 2022, 7:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2022, 7:56 PM
yaxunl requested review of this revision.Mar 15 2022, 7:56 PM
tra added a subscriber: rsmith.Mar 16 2022, 12:37 PM

@rsmith - this touches generic Sema functionality and could use your input.

Overall, the patch looks OK to me.

clang/include/clang/Sema/Sema.h
3327–3328

Perhaps the comment should be restructured so that the cases accected by AllowLambda are described together.

If parsing a lambda, then return X if `AllowLambda`, Y otherwise.
If in 'block' context, return Z.
For regular functions, return W.
yaxunl updated this revision to Diff 416974.Mar 21 2022, 8:56 AM

fix comments

yaxunl marked an inline comment as done.Mar 21 2022, 8:57 AM
yaxunl added inline comments.
clang/include/clang/Sema/Sema.h
3327–3328

done

yaxunl marked an inline comment as done.Mar 23 2022, 8:24 AM

ping

rsmith accepted this revision.Mar 23 2022, 2:01 PM
rsmith added inline comments.
clang/include/clang/Sema/Sema.h
3325–3330

This comment would be clearer if it started with the common case and then explains any special cases. We also prefer to not include the function name at the start of the comment, so that should be removed when updating a function comment. It's also possible for a (member) function to be nested within a lambda, so I don't think it's correct that we will return the lambda whenever we're parsing one. Also, talking about "parsing" isn't right during template instantiation.

Maybe: "Returns a pointer to the innermost enclosing function, or nullptr if the current context is not inside a function. If \p AllowLambda is true, this can return the call operator of an enclosing lambda, otherwise lambdas are skipped when looking for an enclosing function."

clang/test/CodeGenCUDA/openmp-parallel.cu
11
20
This revision is now accepted and ready to land.Mar 23 2022, 2:01 PM
yaxunl marked an inline comment as done.Mar 24 2022, 8:32 AM
yaxunl added inline comments.
clang/include/clang/Sema/Sema.h
3325–3330

Thanks. Will fix when committing.

tra accepted this revision.Mar 24 2022, 10:41 AM
This revision was landed with ongoing or failed builds.Mar 24 2022, 12:20 PM
This revision was automatically updated to reflect the committed changes.
yaxunl marked an inline comment as done.
Herald added a project: Restricted Project. · View Herald TranscriptMar 24 2022, 12:20 PM