[patch] Assert that clang's datalayout strings are in sync with llvm's
Alp Toker
alp at nuanti.com
Thu Dec 19 20:30:18 PST 2013
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
-------------- next part --------------
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;
}
More information about the cfe-commits
mailing list