On 3/6/2013 11:12 AM, Daniel Dunbar wrote:
Hi Matthew,
Is the motivation just as a cleanup?
Primarily cleanup, though we have an internal project that involves
adding a new phase. This makes it a little more straightforward to do so.
Specific comments on the patch:
1. getCompilationPhases should take the phase list as a reference not
a pointer, this is more consistent.
2. Don't .clear() the list in getCompilationPhases.
3. There aren't enough uses to be worth introducing the PhaseList
type, just write out the SmallVector which makes the code more verbose
but more familiar.
New patch attached incorporating this feedback.
Thanks for the review!
Matthew Curtis
- Daniel
On Wed, Mar 6, 2013 at 6:04 AM, Matthew Curtis <[email protected]
<mailto:[email protected]>> wrote:
Hello Daniel,
Do you have time to comment on this patch?
Thanks,
Matthew Curtis.
On 3/1/2013 4:17 PM, Matthew Curtis wrote:
There is now a single function to get the list of phases for a given
output Type.
No functionality change intended.
Thanks,
Matthew Curtis
_______________________________________________
cfe-commits mailing list
[email protected] <mailto:[email protected]>
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation
>From ec5155ae79cb90ff138d9f953948042df7f773de Mon Sep 17 00:00:00 2001
From: Matthew Curtis <[email protected]>
Date: Fri, 1 Mar 2013 15:08:26 -0600
Subject: [PATCH] Minor refactor of how we get compilation phases.
There is now a single function to get the list of phases for a given
output Type.
No functionality change intended.
---
include/clang/Driver/Phases.h | 4 ++
include/clang/Driver/Types.h | 13 ++++----
lib/Driver/Driver.cpp | 17 ++++++-----
lib/Driver/Types.cpp | 62 ++++++++++++++--------------------------
4 files changed, 41 insertions(+), 55 deletions(-)
diff --git a/include/clang/Driver/Phases.h b/include/clang/Driver/Phases.h
index a0c42ea..4e0f40c 100644
--- a/include/clang/Driver/Phases.h
+++ b/include/clang/Driver/Phases.h
@@ -23,6 +23,10 @@ namespace phases {
Link
};
+ enum {
+ MaxNumberOfPhases = Link + 1
+ };
+
const char *getPhaseName(ID Id);
} // end namespace phases
diff --git a/include/clang/Driver/Types.h b/include/clang/Driver/Types.h
index d28ca88..3ca0769 100644
--- a/include/clang/Driver/Types.h
+++ b/include/clang/Driver/Types.h
@@ -11,6 +11,7 @@
#define CLANG_DRIVER_TYPES_H_
#include "clang/Driver/Phases.h"
+#include "llvm/ADT/SmallVector.h"
namespace clang {
namespace driver {
@@ -73,14 +74,12 @@ namespace types {
/// specified type name.
ID lookupTypeForTypeSpecifier(const char *Name);
- /// getNumCompilationPhases - Return the complete number of phases
- /// to be done for this type.
- unsigned getNumCompilationPhases(ID Id);
+ /// getCompilationPhase - Get the list of compilation phases ('Phases') to be
+ /// done for type 'Id'.
+ void getCompilationPhases(
+ ID Id,
+ llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> &Phases);
- /// getCompilationPhase - Return the \p N th compilation phase to
- /// be done for this type.
- phases::ID getCompilationPhase(ID Id, unsigned N);
-
/// lookupCXXTypeForCType - Lookup CXX input type that corresponds to given
/// C type (used for clang++ emulation of g++ behaviour)
ID lookupCXXTypeForCType(ID Id);
diff --git a/lib/Driver/Driver.cpp b/lib/Driver/Driver.cpp
index 8229129..652046f 100644
--- a/lib/Driver/Driver.cpp
+++ b/lib/Driver/Driver.cpp
@@ -1038,17 +1038,17 @@ void Driver::BuildActions(const ToolChain &TC, const
DerivedArgList &Args,
// Construct the actions to perform.
ActionList LinkerInputs;
ActionList SplitInputs;
- unsigned NumSteps = 0;
+ llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> PL;
for (unsigned i = 0, e = Inputs.size(); i != e; ++i) {
types::ID InputType = Inputs[i].first;
const Arg *InputArg = Inputs[i].second;
- NumSteps = types::getNumCompilationPhases(InputType);
- assert(NumSteps && "Invalid number of steps!");
+ PL.clear();
+ types::getCompilationPhases(InputType, PL);
// If the first step comes after the final phase we are doing as part of
// this compilation, warn the user about it.
- phases::ID InitialPhase = types::getCompilationPhase(InputType, 0);
+ phases::ID InitialPhase = PL[0];
if (InitialPhase > FinalPhase) {
// Claim here to avoid the more general unused warning.
InputArg->claim();
@@ -1083,8 +1083,9 @@ void Driver::BuildActions(const ToolChain &TC, const
DerivedArgList &Args,
// Build the pipeline for this file.
OwningPtr<Action> Current(new InputAction(*InputArg, InputType));
- for (unsigned i = 0; i != NumSteps; ++i) {
- phases::ID Phase = types::getCompilationPhase(InputType, i);
+ for (llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases>::iterator
+ i = PL.begin(), e = PL.end(); i != e; ++i) {
+ phases::ID Phase = *i;
// We are done if this step is past what the user requested.
if (Phase > FinalPhase)
@@ -1092,7 +1093,7 @@ void Driver::BuildActions(const ToolChain &TC, const
DerivedArgList &Args,
// Queue linker inputs.
if (Phase == phases::Link) {
- assert(i + 1 == NumSteps && "linking must be final compilation step.");
+ assert((i + 1) == e && "linking must be final compilation step.");
LinkerInputs.push_back(Current.take());
break;
}
@@ -1120,7 +1121,7 @@ void Driver::BuildActions(const ToolChain &TC, const
DerivedArgList &Args,
// If we are linking, claim any options which are obviously only used for
// compilation.
- if (FinalPhase == phases::Link && (NumSteps == 1))
+ if (FinalPhase == phases::Link && PL.size() == 1)
Args.ClaimAllArgs(options::OPT_CompileOnly_Group);
}
diff --git a/lib/Driver/Types.cpp b/lib/Driver/Types.cpp
index 88574fc..9665095 100644
--- a/lib/Driver/Types.cpp
+++ b/lib/Driver/Types.cpp
@@ -179,47 +179,29 @@ types::ID types::lookupTypeForTypeSpecifier(const char
*Name) {
}
// FIXME: Why don't we just put this list in the defs file, eh.
-
-unsigned types::getNumCompilationPhases(ID Id) {
- if (Id == TY_Object)
- return 1;
-
- unsigned N = 0;
- if (getPreprocessedType(Id) != TY_INVALID)
- N += 1;
-
- if (onlyAssembleType(Id))
- return N + 2; // assemble, link
- if (onlyPrecompileType(Id))
- return N + 1; // precompile
-
- return N + 3; // compile, assemble, link
-}
-
-phases::ID types::getCompilationPhase(ID Id, unsigned N) {
- assert(N < getNumCompilationPhases(Id) && "Invalid index.");
-
- if (Id == TY_Object)
- return phases::Link;
-
- if (getPreprocessedType(Id) != TY_INVALID) {
- if (N == 0)
- return phases::Preprocess;
- --N;
+void types::getCompilationPhases(
+ ID Id,
+ llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> &P) {
+ if (Id != TY_Object) {
+ if (getPreprocessedType(Id) != TY_INVALID) {
+ P.push_back(phases::Preprocess);
+ }
+
+ if (onlyPrecompileType(Id)) {
+ P.push_back(phases::Precompile);
+ } else {
+ if (!onlyAssembleType(Id)) {
+ P.push_back(phases::Compile);
+ }
+ P.push_back(phases::Assemble);
+ }
}
-
- if (onlyAssembleType(Id))
- return N == 0 ? phases::Assemble : phases::Link;
-
- if (onlyPrecompileType(Id))
- return phases::Precompile;
-
- if (N == 0)
- return phases::Compile;
- if (N == 1)
- return phases::Assemble;
-
- return phases::Link;
+ if (!onlyPrecompileType(Id)) {
+ P.push_back(phases::Link);
+ }
+ assert(0 < P.size() && "Not enough phases in list");
+ assert(P.size() <= phases::MaxNumberOfPhases && "Too many phases in list");
+ return;
}
ID types::lookupCXXTypeForCType(ID Id) {
--
1.7.8.3
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
