Re: [PATCH] D21840: [Driver][CUDA][OpenMP] Reimplement tool selection in the driver.
ABataev added a comment. LG for me https://reviews.llvm.org/D21840 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21840: [Driver][CUDA][OpenMP] Reimplement tool selection in the driver.
sfantao updated this revision to Diff 72118.
sfantao added a comment.
- Rebase.
https://reviews.llvm.org/D21840
Files:
include/clang/Driver/Action.h
lib/Driver/Driver.cpp
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1922,7 +1922,7 @@
// Create the offload action with all dependences. When an offload action
// is created the kinds are propagated to the host action, so we don't have
-// to do that explicitely here.
+// to do that explicitly here.
OffloadAction::HostDependence HDep(
*HostAction, *C.getSingleOffloadToolChain(),
/*BoundArch*/ nullptr, ActiveOffloadKinds);
@@ -2360,142 +2360,293 @@
}
}
}
-/// Collapse an offloading action looking for a job of the given type. The input
-/// action is changed to the input of the collapsed sequence. If we effectively
-/// had a collapse return the corresponding offloading action, otherwise return
-/// null.
-template
-static OffloadAction *collapseOffloadingAction(Action *) {
- if (!CurAction)
-return nullptr;
- if (auto *OA = dyn_cast(CurAction)) {
-if (OA->hasHostDependence())
- if (auto *HDep = dyn_cast(OA->getHostDependence())) {
-CurAction = HDep;
-return OA;
- }
-if (OA->hasSingleDeviceDependence())
- if (auto *DDep = dyn_cast(OA->getSingleDeviceDependence())) {
-CurAction = DDep;
-return OA;
+
+namespace {
+/// Utility class to control the collapse of dependent actions and select the
+/// tools accordingly.
+class ToolSelector final {
+ /// The tool chain this selector refers to.
+ const ToolChain
+
+ /// The compilation this selector refers to.
+ const Compilation
+
+ /// The base action this selector refers to.
+ const JobAction *BaseAction;
+
+ /// Set to true if the current toolchain refers to host actions.
+ bool IsHostSelector;
+
+ /// Set to true if save-temps and embed-bitcode functionalities are active.
+ bool SaveTemps;
+ bool EmbedBitcode;
+
+ /// Get dependence action or null if that does not exist. If \a CanBeCollapsed
+ /// is false, that action must be legal to collapse or null will be returned.
+ const JobAction *getDependenceAction(const ActionList ,
+ ActionList ,
+ bool CanBeCollapsed = true) {
+// An option can be collapsed only if it has a single input.
+if (Inputs.size() != 1)
+ return nullptr;
+
+Action *CurAction = *Inputs.begin();
+if (!CurAction->isCollapsingWithDependingActionLegal() && CanBeCollapsed)
+ return nullptr;
+
+// If the input action is an offload action. Look through it and save any
+// offload action that can be dropped in the event of a collapse.
+if (auto *OA = dyn_cast(CurAction)) {
+ // If the depending action is a device action, we will attempt to collapse
+ // only with other device actions. Otherwise, we would do the same but
+ // with host actions only.
+ if (!IsHostSelector) {
+if (OA->hasSingleDeviceDependence(/*DoNotConsiderHostActions=*/true)) {
+ CurAction =
+ OA->getSingleDeviceDependence(/*DoNotConsiderHostActions=*/true);
+ if (!CurAction->isCollapsingWithDependingActionLegal() &&
+ CanBeCollapsed)
+return nullptr;
+ SavedOffloadAction.push_back(OA);
+ return dyn_cast(CurAction);
+}
+ } else if (OA->hasHostDependence()) {
+CurAction = OA->getHostDependence();
+if (!CurAction->isCollapsingWithDependingActionLegal() &&
+CanBeCollapsed)
+ return nullptr;
+SavedOffloadAction.push_back(OA);
+return dyn_cast(CurAction);
}
+ return nullptr;
+}
+
+return dyn_cast(CurAction);
}
- return nullptr;
-}
-// Returns a Tool for a given JobAction. In case the action and its
-// predecessors can be combined, updates Inputs with the inputs of the
-// first combined action. If one of the collapsed actions is a
-// CudaHostAction, updates CollapsedCHA with the pointer to it so the
-// caller can deal with extra handling such action requires.
-static const Tool *selectToolForJob(Compilation , bool SaveTemps,
-bool EmbedBitcode, const ToolChain *TC,
-const JobAction *JA,
-const ActionList *,
-ActionList ) {
- const Tool *ToolForJob = nullptr;
- CollapsedOffloadAction.clear();
-
- // See if we should look for a compiler with an integrated assembler. We match
- // bottom up, so what we are actually looking for is an assembler job with a
- // compiler input.
-
- // Look through offload actions between assembler and backend actions.
- Action *BackendJA = (isa(JA) && Inputs->size() == 1)
- ? *Inputs->begin()
-
Re: [PATCH] D21840: [Driver][CUDA][OpenMP] Reimplement tool selection in the driver.
sfantao updated this revision to Diff 66017.
sfantao added a comment.
- Rebase.
https://reviews.llvm.org/D21840
Files:
include/clang/Driver/Action.h
lib/Driver/Driver.cpp
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1898,7 +1898,7 @@
// Create the offload action with all dependences. When an offload action
// is created the kinds are propagated to the host action, so we don't have
-// to do that explicitely here.
+// to do that explicitly here.
OffloadAction::HostDependence HDep(
*HostAction, *C.getSingleOffloadToolChain(),
/*BoundArch*/ nullptr, ActiveOffloadKinds);
@@ -2332,142 +2332,293 @@
}
}
}
-/// Collapse an offloading action looking for a job of the given type. The input
-/// action is changed to the input of the collapsed sequence. If we effectively
-/// had a collapse return the corresponding offloading action, otherwise return
-/// null.
-template
-static OffloadAction *collapseOffloadingAction(Action *) {
- if (!CurAction)
-return nullptr;
- if (auto *OA = dyn_cast(CurAction)) {
-if (OA->hasHostDependence())
- if (auto *HDep = dyn_cast(OA->getHostDependence())) {
-CurAction = HDep;
-return OA;
- }
-if (OA->hasSingleDeviceDependence())
- if (auto *DDep = dyn_cast(OA->getSingleDeviceDependence())) {
-CurAction = DDep;
-return OA;
+
+namespace {
+/// Utility class to control the collapse of dependent actions and select the
+/// tools accordingly.
+class ToolSelector final {
+ /// The tool chain this selector refers to.
+ const ToolChain
+
+ /// The compilation this selector refers to.
+ const Compilation
+
+ /// The base action this selector refers to.
+ const JobAction *BaseAction;
+
+ /// Set to true if the current toolchain refers to host actions.
+ bool IsHostSelector;
+
+ /// Set to true if save-temps and embed-bitcode functionalities are active.
+ bool SaveTemps;
+ bool EmbedBitcode;
+
+ /// Get dependence action or null if that does not exist. If \a CanBeCollapsed
+ /// is false, that action must be legal to collapse or null will be returned.
+ const JobAction *getDependenceAction(const ActionList ,
+ ActionList ,
+ bool CanBeCollapsed = true) {
+// An option can be collapsed only if it has a single input.
+if (Inputs.size() != 1)
+ return nullptr;
+
+Action *CurAction = *Inputs.begin();
+if (!CurAction->isCollapsingWithDependingActionLegal() && CanBeCollapsed)
+ return nullptr;
+
+// If the input action is an offload action. Look through it and save any
+// offload action that can be dropped in the event of a collapse.
+if (auto *OA = dyn_cast(CurAction)) {
+ // If the depending action is a device action, we will attempt to collapse
+ // only with other device actions. Otherwise, we would do the same but
+ // with host actions only.
+ if (!IsHostSelector) {
+if (OA->hasSingleDeviceDependence(/*DoNotConsiderHostActions=*/true)) {
+ CurAction =
+ OA->getSingleDeviceDependence(/*DoNotConsiderHostActions=*/true);
+ if (!CurAction->isCollapsingWithDependingActionLegal() &&
+ CanBeCollapsed)
+return nullptr;
+ SavedOffloadAction.push_back(OA);
+ return dyn_cast(CurAction);
+}
+ } else if (OA->hasHostDependence()) {
+CurAction = OA->getHostDependence();
+if (!CurAction->isCollapsingWithDependingActionLegal() &&
+CanBeCollapsed)
+ return nullptr;
+SavedOffloadAction.push_back(OA);
+return dyn_cast(CurAction);
}
+ return nullptr;
+}
+
+return dyn_cast(CurAction);
}
- return nullptr;
-}
-// Returns a Tool for a given JobAction. In case the action and its
-// predecessors can be combined, updates Inputs with the inputs of the
-// first combined action. If one of the collapsed actions is a
-// CudaHostAction, updates CollapsedCHA with the pointer to it so the
-// caller can deal with extra handling such action requires.
-static const Tool *selectToolForJob(Compilation , bool SaveTemps,
-bool EmbedBitcode, const ToolChain *TC,
-const JobAction *JA,
-const ActionList *,
-ActionList ) {
- const Tool *ToolForJob = nullptr;
- CollapsedOffloadAction.clear();
-
- // See if we should look for a compiler with an integrated assembler. We match
- // bottom up, so what we are actually looking for is an assembler job with a
- // compiler input.
-
- // Look through offload actions between assembler and backend actions.
- Action *BackendJA = (isa(JA) && Inputs->size() == 1)
- ? *Inputs->begin()
-
Re: [PATCH] D21840: [Driver][CUDA][OpenMP] Reimplement tool selection in the driver.
sfantao updated this revision to Diff 63682.
sfantao added a comment.
- Rebase.
http://reviews.llvm.org/D21840
Files:
include/clang/Driver/Action.h
lib/Driver/Driver.cpp
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1898,7 +1898,7 @@
// Create the offload action with all dependences. When an offload action
// is created the kinds are propagated to the host action, so we don't have
-// to do that explicitely here.
+// to do that explicitly here.
OffloadAction::HostDependence HDep(
*HostAction, *C.getSingleOffloadToolChain(),
/*BoundArch*/ nullptr, ActiveOffloadKinds);
@@ -2332,142 +2332,293 @@
}
}
}
-/// Collapse an offloading action looking for a job of the given type. The input
-/// action is changed to the input of the collapsed sequence. If we effectively
-/// had a collapse return the corresponding offloading action, otherwise return
-/// null.
-template
-static OffloadAction *collapseOffloadingAction(Action *) {
- if (!CurAction)
-return nullptr;
- if (auto *OA = dyn_cast(CurAction)) {
-if (OA->hasHostDependence())
- if (auto *HDep = dyn_cast(OA->getHostDependence())) {
-CurAction = HDep;
-return OA;
- }
-if (OA->hasSingleDeviceDependence())
- if (auto *DDep = dyn_cast(OA->getSingleDeviceDependence())) {
-CurAction = DDep;
-return OA;
+
+namespace {
+/// Utility class to control the collapse of dependent actions and select the
+/// tools accordingly.
+class ToolSelector final {
+ /// The tool chain this selector refers to.
+ const ToolChain
+
+ /// The compilation this selector refers to.
+ const Compilation
+
+ /// The base action this selector refers to.
+ const JobAction *BaseAction;
+
+ /// Set to true if the current toolchain refers to host actions.
+ bool IsHostSelector;
+
+ /// Set to true if save-temps and embed-bitcode functionalities are active.
+ bool SaveTemps;
+ bool EmbedBitcode;
+
+ /// Get dependence action or null if that does not exist. If \a CanBeCollapsed
+ /// is false, that action must be legal to collapse or null will be returned.
+ const JobAction *getDependenceAction(const ActionList ,
+ ActionList ,
+ bool CanBeCollapsed = true) {
+// An option can be collapsed only if it has a single input.
+if (Inputs.size() != 1)
+ return nullptr;
+
+Action *CurAction = *Inputs.begin();
+if (!CurAction->isCollapsingWithDependingActionLegal() && CanBeCollapsed)
+ return nullptr;
+
+// If the input action is an offload action. Look through it and save any
+// offload action that can be dropped in the event of a collapse.
+if (auto *OA = dyn_cast(CurAction)) {
+ // If the depending action is a device action, we will attempt to collapse
+ // only with other device actions. Otherwise, we would do the same but
+ // with host actions only.
+ if (!IsHostSelector) {
+if (OA->hasSingleDeviceDependence(/*DoNotConsiderHostActions=*/true)) {
+ CurAction =
+ OA->getSingleDeviceDependence(/*DoNotConsiderHostActions=*/true);
+ if (!CurAction->isCollapsingWithDependingActionLegal() &&
+ CanBeCollapsed)
+return nullptr;
+ SavedOffloadAction.push_back(OA);
+ return dyn_cast(CurAction);
+}
+ } else if (OA->hasHostDependence()) {
+CurAction = OA->getHostDependence();
+if (!CurAction->isCollapsingWithDependingActionLegal() &&
+CanBeCollapsed)
+ return nullptr;
+SavedOffloadAction.push_back(OA);
+return dyn_cast(CurAction);
}
+ return nullptr;
+}
+
+return dyn_cast(CurAction);
}
- return nullptr;
-}
-// Returns a Tool for a given JobAction. In case the action and its
-// predecessors can be combined, updates Inputs with the inputs of the
-// first combined action. If one of the collapsed actions is a
-// CudaHostAction, updates CollapsedCHA with the pointer to it so the
-// caller can deal with extra handling such action requires.
-static const Tool *selectToolForJob(Compilation , bool SaveTemps,
-bool EmbedBitcode, const ToolChain *TC,
-const JobAction *JA,
-const ActionList *,
-ActionList ) {
- const Tool *ToolForJob = nullptr;
- CollapsedOffloadAction.clear();
-
- // See if we should look for a compiler with an integrated assembler. We match
- // bottom up, so what we are actually looking for is an assembler job with a
- // compiler input.
-
- // Look through offload actions between assembler and backend actions.
- Action *BackendJA = (isa(JA) && Inputs->size() == 1)
- ? *Inputs->begin()
-
Re: [PATCH] D21840: [Driver][CUDA][OpenMP] Reimplement tool selection in the driver.
sfantao updated this revision to Diff 62575.
sfantao added a comment.
- Rebase
http://reviews.llvm.org/D21840
Files:
include/clang/Driver/Action.h
lib/Driver/Driver.cpp
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1894,7 +1894,7 @@
// Create the offload action with all dependences. When an offload action
// is created the kinds are propagated to the host action, so we don't have
-// to do that explicitely here.
+// to do that explicitly here.
OffloadAction::HostDependence HDep(
*HostAction, *C.getSingleOffloadToolChain(),
/*BoundArch*/ nullptr, ActiveOffloadKinds);
@@ -2328,142 +2328,293 @@
}
}
}
-/// Collapse an offloading action looking for a job of the given type. The input
-/// action is changed to the input of the collapsed sequence. If we effectively
-/// had a collapse return the corresponding offloading action, otherwise return
-/// null.
-template
-static OffloadAction *collapseOffloadingAction(Action *) {
- if (!CurAction)
-return nullptr;
- if (auto *OA = dyn_cast(CurAction)) {
-if (OA->hasHostDependence())
- if (auto *HDep = dyn_cast(OA->getHostDependence())) {
-CurAction = HDep;
-return OA;
- }
-if (OA->hasSingleDeviceDependence())
- if (auto *DDep = dyn_cast(OA->getSingleDeviceDependence())) {
-CurAction = DDep;
-return OA;
+
+namespace {
+/// Utility class to control the collapse of dependent actions and select the
+/// tools accordingly.
+class ToolSelector final {
+ /// The tool chain this selector refers to.
+ const ToolChain
+
+ /// The compilation this selector refers to.
+ const Compilation
+
+ /// The base action this selector refers to.
+ const JobAction *BaseAction;
+
+ /// Set to true if the current toolchain refers to host actions.
+ bool IsHostSelector;
+
+ /// Set to true if save-temps and embed-bitcode functionalities are active.
+ bool SaveTemps;
+ bool EmbedBitcode;
+
+ /// Get dependence action or null if that does not exist. If \a CanBeCollapsed
+ /// is false, that action must be legal to collapse or null will be returned.
+ const JobAction *getDependenceAction(const ActionList ,
+ ActionList ,
+ bool CanBeCollapsed = true) {
+// An option can be collapsed only if it has a single input.
+if (Inputs.size() != 1)
+ return nullptr;
+
+Action *CurAction = *Inputs.begin();
+if (!CurAction->isCollapsingWithDependingActionLegal() && CanBeCollapsed)
+ return nullptr;
+
+// If the input action is an offload action. Look through it and save any
+// offload action that can be dropped in the event of a collapse.
+if (auto *OA = dyn_cast(CurAction)) {
+ // If the depending action is a device action, we will attempt to collapse
+ // only with other device actions. Otherwise, we would do the same but
+ // with host actions only.
+ if (!IsHostSelector) {
+if (OA->hasSingleDeviceDependence(/*DoNotConsiderHostActions=*/true)) {
+ CurAction =
+ OA->getSingleDeviceDependence(/*DoNotConsiderHostActions=*/true);
+ if (!CurAction->isCollapsingWithDependingActionLegal() &&
+ CanBeCollapsed)
+return nullptr;
+ SavedOffloadAction.push_back(OA);
+ return dyn_cast(CurAction);
+}
+ } else if (OA->hasHostDependence()) {
+CurAction = OA->getHostDependence();
+if (!CurAction->isCollapsingWithDependingActionLegal() &&
+CanBeCollapsed)
+ return nullptr;
+SavedOffloadAction.push_back(OA);
+return dyn_cast(CurAction);
}
+ return nullptr;
+}
+
+return dyn_cast(CurAction);
}
- return nullptr;
-}
-// Returns a Tool for a given JobAction. In case the action and its
-// predecessors can be combined, updates Inputs with the inputs of the
-// first combined action. If one of the collapsed actions is a
-// CudaHostAction, updates CollapsedCHA with the pointer to it so the
-// caller can deal with extra handling such action requires.
-static const Tool *selectToolForJob(Compilation , bool SaveTemps,
-bool EmbedBitcode, const ToolChain *TC,
-const JobAction *JA,
-const ActionList *,
-ActionList ) {
- const Tool *ToolForJob = nullptr;
- CollapsedOffloadAction.clear();
-
- // See if we should look for a compiler with an integrated assembler. We match
- // bottom up, so what we are actually looking for is an assembler job with a
- // compiler input.
-
- // Look through offload actions between assembler and backend actions.
- Action *BackendJA = (isa(JA) && Inputs->size() == 1)
- ? *Inputs->begin()
-
Re: [PATCH] D21840: [Driver][CUDA][OpenMP] Reimplement tool selection in the driver.
sfantao added a comment.
Hi Alexey,
Thanks for the review!
Comment at: lib/Driver/Driver.cpp:2416
@@ +2415,3 @@
+ /// be collapsed with it.
+ struct JobActionInfoTy {
+// \brief The action this info refers to.
ABataev wrote:
> I think this can be marked 'final' and add 'nullptr' as a default initializer
> for 'JA' field
Yep, I'm doing that now.
Comment at: lib/Driver/Driver.cpp:2426
@@ +2425,3 @@
+ /// in the action info array.
+ void appendCollapsedOffloadAction(ActionList ,
+ArrayRef ,
ABataev wrote:
> I think this function can be marked as 'static', no?
You're right, using static now.
http://reviews.llvm.org/D21840
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D21840: [Driver][CUDA][OpenMP] Reimplement tool selection in the driver.
sfantao updated this revision to Diff 62507.
sfantao marked 7 inline comments as done.
sfantao added a comment.
- Fix comments.
- Mark functions properly with const and static.
- Remove \brief.
http://reviews.llvm.org/D21840
Files:
include/clang/Driver/Action.h
lib/Driver/Driver.cpp
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -1894,7 +1894,7 @@
// Create the offload action with all dependences. When an offload action
// is created the kinds are propagated to the host action, so we don't have
-// to do that explicitely here.
+// to do that explicitly here.
OffloadAction::HostDependence HDep(
*HostAction, *C.getSingleOffloadToolChain(),
/*BoundArch*/ nullptr, ActiveOffloadKinds);
@@ -2328,142 +2328,292 @@
}
}
}
-/// Collapse an offloading action looking for a job of the given type. The input
-/// action is changed to the input of the collapsed sequence. If we effectively
-/// had a collapse return the corresponding offloading action, otherwise return
-/// null.
-template
-static OffloadAction *collapseOffloadingAction(Action *) {
- if (!CurAction)
-return nullptr;
- if (auto *OA = dyn_cast(CurAction)) {
-if (OA->hasHostDependence())
- if (auto *HDep = dyn_cast(OA->getHostDependence())) {
-CurAction = HDep;
-return OA;
- }
-if (OA->hasSingleDeviceDependence())
- if (auto *DDep = dyn_cast(OA->getSingleDeviceDependence())) {
-CurAction = DDep;
-return OA;
+
+namespace {
+/// Utility class to control the collapse of dependent actions and select the
+/// tools accordingly.
+class ToolSelector final {
+ /// The tool chain this selector refers to.
+ const ToolChain
+
+ /// The compilation this selector refers to.
+ const Compilation
+
+ /// The base action this selector refers to.
+ const JobAction *BaseAction;
+
+ /// Set to true if the current toolchain refers to host actions.
+ bool IsHostSelector;
+
+ /// Set to true if save-temps and embed-bitcode functionalities are active.
+ bool SaveTemps;
+ bool EmbedBitcode;
+
+ /// Get dependence action or null if that does not exist. If \a CanBeCollapsed
+ /// is false, that action must be legal to collapse or null will be returned.
+ const JobAction *getDependenceAction(const ActionList ,
+ ActionList ,
+ bool CanBeCollapsed = true) {
+// An option can be collapsed only if it has a single input.
+if (Inputs.size() != 1)
+ return nullptr;
+
+Action *CurAction = *Inputs.begin();
+if (!CurAction->isCollapsingWithDependingActionLegal() && CanBeCollapsed)
+ return nullptr;
+
+// If the input action is an offload action. Look through it and save any
+// offload action that can be dropped in the event of a collapse.
+if (auto *OA = dyn_cast(CurAction)) {
+ // If the depending action is a device action, we will attempt to collapse
+ // only with other device actions. Otherwise, we would do the same but
+ // with host actions only.
+ if (!IsHostSelector) {
+if (OA->hasSingleDeviceDependence(/*DoNotConsiderHostActions=*/true)) {
+ CurAction =
+ OA->getSingleDeviceDependence(/*DoNotConsiderHostActions=*/true);
+ if (!CurAction->isCollapsingWithDependingActionLegal() &&
+ CanBeCollapsed)
+return nullptr;
+ SavedOffloadAction.push_back(OA);
+ return dyn_cast(CurAction);
+}
+ } else if (OA->hasHostDependence()) {
+CurAction = OA->getHostDependence();
+if (!CurAction->isCollapsingWithDependingActionLegal() &&
+CanBeCollapsed)
+ return nullptr;
+SavedOffloadAction.push_back(OA);
+return dyn_cast(CurAction);
}
+ return nullptr;
+}
+
+return dyn_cast(CurAction);
}
- return nullptr;
-}
-// Returns a Tool for a given JobAction. In case the action and its
-// predecessors can be combined, updates Inputs with the inputs of the
-// first combined action. If one of the collapsed actions is a
-// CudaHostAction, updates CollapsedCHA with the pointer to it so the
-// caller can deal with extra handling such action requires.
-static const Tool *selectToolForJob(Compilation , bool SaveTemps,
-bool EmbedBitcode, const ToolChain *TC,
-const JobAction *JA,
-const ActionList *,
-ActionList ) {
- const Tool *ToolForJob = nullptr;
- CollapsedOffloadAction.clear();
-
- // See if we should look for a compiler with an integrated assembler. We match
- // bottom up, so what we are actually looking for is an assembler job with a
- // compiler input.
-
- // Look through offload actions between assembler and backend
Re: [PATCH] D21840: [Driver][CUDA][OpenMP] Reimplement tool selection in the driver.
ABataev added a comment.
General comment: remove '\brief' tags, they are not required anymore, just
'\\\' is enough
Comment at: include/clang/Driver/Action.h:94
@@ -93,1 +93,3 @@
+ /// \brief Flag that is set to true if this action can be collapsed with
+ /// others actions that depend on it. This is true by default and set to
false
'\brief' tag is not required, remove it.
Comment at: lib/Driver/Driver.cpp:2400
@@ +2399,3 @@
+ /// \brief Return true if an assemble action can be collapsed.
+ bool canCollapseAssembleAction() {
+return TC.useIntegratedAs() && !SaveTemps &&
bool canCollapseAssembleAction() -> bool canCollapseAssembleAction() const
Comment at: lib/Driver/Driver.cpp:2408
@@ +2407,3 @@
+ /// \brief Return true if a preprocessor action can be collapsed.
+ bool canCollapsePreprocessorAction() {
+return !C.getArgs().hasArg(options::OPT_no_integrated_cpp) &&
Also can be marked as const
Comment at: lib/Driver/Driver.cpp:2416
@@ +2415,3 @@
+ /// be collapsed with it.
+ struct JobActionInfoTy {
+// \brief The action this info refers to.
I think this can be marked 'final' and add 'nullptr' as a default initializer
for 'JA' field
Comment at: lib/Driver/Driver.cpp:2424
@@ +2423,3 @@
+
+ /// \brief Append collapsed offload actions from the give nnumber of elements
+ /// in the action info array.
Remove '\brief'
Comment at: lib/Driver/Driver.cpp:2426
@@ +2425,3 @@
+ /// in the action info array.
+ void appendCollapsedOffloadAction(ActionList ,
+ArrayRef ,
I think this function can be marked as 'static', no?
Comment at: lib/Driver/Driver.cpp:2435
@@ +2434,3 @@
+
+ /// \brief Functions that attempt to perform the combining. They detect if
+ /// that is legal, and if so they update the inputs \a Inputs and the offload
remove '\brief'
http://reviews.llvm.org/D21840
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
