This is an archive of the discontinued LLVM Phabricator instance.

[OpenMP] Extend NVPTX SPMD implementation of combined constructs
ClosedPublic

Authored by carlo.bertolli on Feb 27 2018, 6:45 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

carlo.bertolli created this revision.Feb 27 2018, 6:45 PM
ABataev accepted this revision.Feb 28 2018, 6:26 AM

LG, with some nits

include/clang/Driver/Options.td
1428 ↗(On Diff #136214)

This flag also must be CC1Option

lib/Frontend/CompilerInvocation.cpp
2533 ↗(On Diff #136214)

After some thoughts I think it is better to make true by default, because Generic mode is not completed yet.

This revision is now accepted and ready to land.Feb 28 2018, 6:26 AM
Hahnfeld added inline comments.
lib/Driver/ToolChains/Clang.cpp
3976–3977 ↗(On Diff #136214)

I think most other boolean options do the following:

if (Args.hasFlag(...))
  CmdArgs.push_back("...")

Is there a reason we need this differently here?

lib/Frontend/CompilerInvocation.cpp
2533 ↗(On Diff #136214)

Yes, I'd also expect all SPMD constructs to default to CUDA mode. Or is there a case where this doesn't work? If yes, that should be explained in the summary.

ABataev added inline comments.Feb 28 2018, 7:02 AM
include/clang/Driver/Options.td
1428 ↗(On Diff #136214)

If you implement Jonas's suggested fix, no need to mark it as CC1Option

lib/Driver/ToolChains/Clang.cpp
3976–3977 ↗(On Diff #136214)

Agree, this looks much better

lib/Frontend/CompilerInvocation.cpp
2533 ↗(On Diff #136214)

Cuda mode is going to be the default for all constructs, as Generic mode is not ready yet. The codegen mode is not controlled by the construct, but by the option completely.

This revision was automatically updated to reflect the committed changes.