On 20/12/2013 00:03, Rafael Espíndola wrote:
Hi Alp,
This depends on
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20131216/199362.html,
but I am sending it now since the code review itself can probably
happen in parallel.
I noticed that clang and llvm datalayout strings were out of sync. I
have manually synchronized them (other than the pending ARM review),
but they are likely to get out of sync again if nothing is done about
it.
Thanks Rafael, great work syncing up the strings. If you didn't do this
I don't know who would.
There are 4 special cases that are really easy: le32, spir, spir64 and
tce. They are easy because they don't exist in LLVM, so the only
reasonable solution is to keep them in clang. It would be quite
burdensome to ask them to have a pseudo llvm target just to get the
string.
Agreed.
For the remaining cases my original intent was to drop them from clang
and have clang ask llvm. As you noted on IRC, this would have the
disadvantage of requiring a llvm target to be linked in to be able to
produce IR for that target.
What the attached patch does is just assert that, if a llvm backend is
available, its string matches the one in clang.
The idea is fantastic, but I found your implementation burdensome.
Bringing a TargetMachine dependency into IRGen, even if it's a "soft"
dependency is best avoided because the clear layering between IRGen and
CodeGen is LLVM's stated foremost design goal.
The extraction of CreateTargetMachine() into a long list of parameters
in your patch also left something to be desired and we should be able to
avoid it.
It might be possible to still refactor things to avoid the
duplication, but for now this should at least make sure both projects
stay in sync.
I've attached my take on your idea, implementing it in a net 8 LoC.
In my patch, the verification is done at IRGen so there's no runtime
penalty, and moreover no change in layering.
Even with this verification done, we should continue to investigate
options for sharing the layouts and getting them threaded through the
frontend. Two copies are one too many.
Alp.
Cheers,
Rafael
--
http://www.nuanti.com
the browser experts
diff --git a/include/clang/CodeGen/BackendUtil.h
b/include/clang/CodeGen/BackendUtil.h
index b9f352c..f8b90b3 100644
--- a/include/clang/CodeGen/BackendUtil.h
+++ b/include/clang/CodeGen/BackendUtil.h
@@ -33,8 +33,8 @@ namespace clang {
void EmitBackendOutput(DiagnosticsEngine &Diags, const CodeGenOptions
&CGOpts,
const TargetOptions &TOpts, const LangOptions &LOpts,
- llvm::Module *M,
- BackendAction Action, raw_ostream *OS);
+ StringRef TDesc, llvm::Module *M, BackendAction
Action,
+ raw_ostream *OS);
}
#endif
diff --git a/lib/CodeGen/BackendUtil.cpp b/lib/CodeGen/BackendUtil.cpp
index 90b0f68..fa89234 100644
--- a/lib/CodeGen/BackendUtil.cpp
+++ b/lib/CodeGen/BackendUtil.cpp
@@ -55,7 +55,7 @@ class EmitAssemblyHelper {
mutable FunctionPassManager *PerFunctionPasses;
private:
- PassManager *getCodeGenPasses(TargetMachine *TM) const {
+ PassManager *getCodeGenPasses() const {
if (!CodeGenPasses) {
CodeGenPasses = new PassManager();
CodeGenPasses->add(new DataLayout(TheModule));
@@ -65,7 +65,7 @@ private:
return CodeGenPasses;
}
- PassManager *getPerModulePasses(TargetMachine *TM) const {
+ PassManager *getPerModulePasses() const {
if (!PerModulePasses) {
PerModulePasses = new PassManager();
PerModulePasses->add(new DataLayout(TheModule));
@@ -75,7 +75,7 @@ private:
return PerModulePasses;
}
- FunctionPassManager *getPerFunctionPasses(TargetMachine *TM) const {
+ FunctionPassManager *getPerFunctionPasses() const {
if (!PerFunctionPasses) {
PerFunctionPasses = new FunctionPassManager(TheModule);
PerFunctionPasses->add(new DataLayout(TheModule));
@@ -86,7 +86,7 @@ private:
}
- void CreatePasses(TargetMachine *TM);
+ void CreatePasses();
/// CreateTargetMachine - Generates the TargetMachine.
/// Returns Null if it is unable to create the target machine.
@@ -101,8 +101,7 @@ private:
/// AddEmitPasses - Add passes necessary to emit assembly or LLVM IR.
///
/// \return True on success.
- bool AddEmitPasses(BackendAction Action, formatted_raw_ostream &OS,
- TargetMachine *TM);
+ bool AddEmitPasses(BackendAction Action, formatted_raw_ostream &OS);
public:
EmitAssemblyHelper(DiagnosticsEngine &_Diags,
@@ -118,8 +117,12 @@ public:
delete CodeGenPasses;
delete PerModulePasses;
delete PerFunctionPasses;
+ if (CodeGenOpts.DisableFree)
+ TM.take();
}
+ llvm::OwningPtr<TargetMachine> TM;
+
void EmitAssembly(BackendAction Action, raw_ostream *OS);
};
@@ -222,7 +225,7 @@ static void addDataFlowSanitizerPass(const
PassManagerBuilder &Builder,
PM.add(createDataFlowSanitizerPass(CGOpts.SanitizerBlacklistFile));
}
-void EmitAssemblyHelper::CreatePasses(TargetMachine *TM) {
+void EmitAssemblyHelper::CreatePasses() {
unsigned OptLevel = CodeGenOpts.OptimizationLevel;
CodeGenOptions::InliningMethod Inlining = CodeGenOpts.getInlining();
@@ -324,13 +327,13 @@ void EmitAssemblyHelper::CreatePasses(TargetMachine *TM) {
}
// Set up the per-function pass manager.
- FunctionPassManager *FPM = getPerFunctionPasses(TM);
+ FunctionPassManager *FPM = getPerFunctionPasses();
if (CodeGenOpts.VerifyModule)
FPM->add(createVerifierPass());
PMBuilder.populateFunctionPassManager(*FPM);
// Set up the per-module pass manager.
- PassManager *MPM = getPerModulePasses(TM);
+ PassManager *MPM = getPerModulePasses();
if (!CodeGenOpts.DisableGCov &&
(CodeGenOpts.EmitGcovArcs || CodeGenOpts.EmitGcovNotes)) {
@@ -503,11 +506,10 @@ TargetMachine
*EmitAssemblyHelper::CreateTargetMachine(bool MustCreateTM) {
}
bool EmitAssemblyHelper::AddEmitPasses(BackendAction Action,
- formatted_raw_ostream &OS,
- TargetMachine *TM) {
+ formatted_raw_ostream &OS) {
// Create the code generator passes.
- PassManager *PM = getCodeGenPasses(TM);
+ PassManager *PM = getCodeGenPasses();
// Add LibraryInfo.
llvm::Triple TargetTriple(TheModule->getTargetTriple());
@@ -552,27 +554,27 @@ void EmitAssemblyHelper::EmitAssembly(BackendAction
Action, raw_ostream *OS) {
bool UsesCodeGen = (Action != Backend_EmitNothing &&
Action != Backend_EmitBC &&
Action != Backend_EmitLL);
- TargetMachine *TM = CreateTargetMachine(UsesCodeGen);
+ if (!TM) TM.reset(CreateTargetMachine(UsesCodeGen));
+
if (UsesCodeGen && !TM) return;
- llvm::OwningPtr<TargetMachine> TMOwner(CodeGenOpts.DisableFree ? 0 : TM);
- CreatePasses(TM);
+ CreatePasses();
switch (Action) {
case Backend_EmitNothing:
break;
case Backend_EmitBC:
- getPerModulePasses(TM)->add(createBitcodeWriterPass(*OS));
+ getPerModulePasses()->add(createBitcodeWriterPass(*OS));
break;
case Backend_EmitLL:
FormattedOS.setStream(*OS, formatted_raw_ostream::PRESERVE_STREAM);
- getPerModulePasses(TM)->add(createPrintModulePass(&FormattedOS));
+ getPerModulePasses()->add(createPrintModulePass(&FormattedOS));
break;
default:
FormattedOS.setStream(*OS, formatted_raw_ostream::PRESERVE_STREAM);
- if (!AddEmitPasses(Action, FormattedOS, TM))
+ if (!AddEmitPasses(Action, FormattedOS))
return;
}
@@ -607,10 +609,15 @@ void EmitAssemblyHelper::EmitAssembly(BackendAction
Action, raw_ostream *OS) {
void clang::EmitBackendOutput(DiagnosticsEngine &Diags,
const CodeGenOptions &CGOpts,
const clang::TargetOptions &TOpts,
- const LangOptions &LOpts,
- Module *M,
- BackendAction Action, raw_ostream *OS) {
+ const LangOptions &LOpts, StringRef TDesc,
+ Module *M, BackendAction Action,
+ raw_ostream *OS) {
EmitAssemblyHelper AsmHelper(Diags, CGOpts, TOpts, LOpts, M);
AsmHelper.EmitAssembly(Action, OS);
+
+ // If an optional clang TargetInfo description string was passed in, use it
to
+ // verify the LLVM TargetMachine's DataLayout.
+ assert(TDesc.empty() || !AsmHelper.TM ||
+ TDesc == AsmHelper.TM->getDataLayout()->getStringRepresentation());
}
diff --git a/lib/CodeGen/CodeGenAction.cpp b/lib/CodeGen/CodeGenAction.cpp
index 047dc53..520f774 100644
--- a/lib/CodeGen/CodeGenAction.cpp
+++ b/lib/CodeGen/CodeGenAction.cpp
@@ -150,6 +150,7 @@ namespace clang {
Ctx.setInlineAsmDiagnosticHandler(InlineAsmDiagHandler, this);
EmitBackendOutput(Diags, CodeGenOpts, TargetOpts, LangOpts,
+ C.getTargetInfo().getTargetDescription(),
TheModule.get(), Action, AsmOutStream);
Ctx.setInlineAsmDiagnosticHandler(OldHandler, OldContext);
@@ -422,7 +423,7 @@ void CodeGenAction::ExecuteAction() {
EmitBackendOutput(CI.getDiagnostics(), CI.getCodeGenOpts(),
CI.getTargetOpts(), CI.getLangOpts(),
- TheModule.get(),
+ CI.getTarget().getTargetDescription(), TheModule.get(),
BA, OS);
return;
}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
