[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

[todd toddcarMAY via Phabricator via cfe-commits]

toddcarmay added a comment.
Herald added a project: All.

I've been trying to find anything like this for a while now. Thanks for posting 
this, I've been hunting for it for a while. https://drift-boss.co/ drift boss


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113352/new/

https://reviews.llvm.org/D113352

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

[Itay Bookstein via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9efce0baee4b: [clang] Run LLVM Verifier in modes without 
CodeGen too (authored by ibookstein).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113352/new/

https://reviews.llvm.org/D113352

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/lto-newpm-pipeline.c


Index: clang/test/CodeGen/lto-newpm-pipeline.c
===
--- clang/test/CodeGen/lto-newpm-pipeline.c
+++ clang/test/CodeGen/lto-newpm-pipeline.c
@@ -32,6 +32,8 @@
 // CHECK-FULL-O0-NEXT: Running pass: CoroCleanupPass
 // CHECK-FULL-O0-NEXT: Running pass: CanonicalizeAliasesPass
 // CHECK-FULL-O0-NEXT: Running pass: NameAnonGlobalPass
+// CHECK-FULL-O0-NEXT: Running pass: VerifierPass
+// CHECK-FULL-O0-NEXT: Running analysis: VerifierAnalysis
 // CHECK-FULL-O0-NEXT: Running pass: BitcodeWriterPass
 
 // CHECK-THIN-O0: Running pass: AlwaysInlinerPass
@@ -40,6 +42,8 @@
 // CHECK-THIN-O0: Running pass: CoroCleanupPass
 // CHECK-THIN-O0-NEXT: Running pass: CanonicalizeAliasesPass
 // CHECK-THIN-O0-NEXT: Running pass: NameAnonGlobalPass
+// CHECK-THIN-O0-NEXT: Running pass: VerifierPass
+// CHECK-THIN-O0-NEXT: Running analysis: VerifierAnalysis
 // CHECK-THIN-O0-NEXT: Running pass: ThinLTOBitcodeWriterPass
 
 // TODO: The LTO pre-link pipeline currently invokes
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5034,7 +5034,7 @@
   llvm::Type *DeclTy = getTypes().ConvertTypeForMem(D->getType());
   llvm::Type *ResolverTy = llvm::GlobalIFunc::getResolverFunctionType(DeclTy);
   llvm::Constant *Resolver =
-  GetOrCreateLLVMFunction(IFA->getResolver(), ResolverTy, GD,
+  GetOrCreateLLVMFunction(IFA->getResolver(), ResolverTy, {},
   /*ForVTable=*/false);
   llvm::GlobalIFunc *GIF =
   llvm::GlobalIFunc::create(DeclTy, 0, llvm::Function::ExternalLinkage,
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -487,6 +487,11 @@
   }
 }
 
+static bool actionRequiresCodeGen(BackendAction Action) {
+  return Action != Backend_EmitNothing && Action != Backend_EmitBC &&
+ Action != Backend_EmitLL;
+}
+
 static bool initTargetOptions(DiagnosticsEngine ,
   llvm::TargetOptions ,
   const CodeGenOptions ,
@@ -977,9 +982,7 @@
 
   setCommandLineOpts(CodeGenOpts);
 
-  bool UsesCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC &&
-  Action != Backend_EmitLL);
+  bool UsesCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(UsesCodeGen);
 
   if (UsesCodeGen && !TM)
@@ -1006,6 +1009,12 @@
 
   CreatePasses(PerModulePasses, PerFunctionPasses);
 
+  // Add a verifier pass if requested. We don't have to do this if the action
+  // requires code generation because there will already be a verifier pass in
+  // the code-generation pipeline.
+  if (!UsesCodeGen && CodeGenOpts.VerifyModule)
+PerModulePasses.add(createVerifierPass());
+
   legacy::PassManager CodeGenPasses;
   CodeGenPasses.add(
   createTargetTransformInfoWrapperPass(getTargetIRAnalysis()));
@@ -1425,6 +1434,13 @@
   MPM.addPass(ModuleMemProfilerPass());
 }
   }
+
+  // Add a verifier pass if requested. We don't have to do this if the action
+  // requires code generation because there will already be a verifier pass in
+  // the code-generation pipeline.
+  if (!actionRequiresCodeGen(Action) && CodeGenOpts.VerifyModule)
+MPM.addPass(VerifierPass());
+
   switch (Action) {
   case Backend_EmitBC:
 if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
@@ -1514,8 +1530,7 @@
   TimeRegion Region(CodeGenOpts.TimePasses ?  : nullptr);
   setCommandLineOpts(CodeGenOpts);
 
-  bool RequiresCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC && Action != 
Backend_EmitLL);
+  bool RequiresCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(RequiresCodeGen);
 
   if (RequiresCodeGen && !TM)


Index: clang/test/CodeGen/lto-newpm-pipeline.c
===
--- clang/test/CodeGen/lto-newpm-pipeline.c
+++ clang/test/CodeGen/lto-newpm-pipeline.c
@@ -32,6 +32,8 @@
 // CHECK-FULL-O0-NEXT: Running pass: CoroCleanupPass
 // CHECK-FULL-O0-NEXT: Running pass: CanonicalizeAliasesPass
 // CHECK-FULL-O0-NEXT: Running pass: NameAnonGlobalPass
+// CHECK-FULL-O0-NEXT: Running pass: VerifierPass
+// CHECK-FULL-O0-NEXT: Running analysis: VerifierAnalysis
 // CHECK-FULL-O0-NEXT: Running pass: 

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

Thanks; Might I ask that you commit this on my behalf? :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113352/new/

https://reviews.llvm.org/D113352

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

[John McCall via Phabricator via cfe-commits]

rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

LGTM




Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5016
   llvm::Constant *Resolver =
-  GetOrCreateLLVMFunction(IFA->getResolver(), ResolverTy, GD,
+  GetOrCreateLLVMFunction(IFA->getResolver(), ResolverTy, {},
   /*ForVTable=*/false);

ibookstein wrote:
> rjmccall wrote:
> > Hmm, what was going on here?
> The `emitIFuncDefinition` fucntion incorrectly passes the GlobalDecl of the 
> IFunc itself to the call to GetOrCreateLLVMFunction for creating the 
> resolver, which causes it to be created with a wrong attribute list, which 
> fails `Verifier::visitFunction` with "Attribute after last parameter!". 
> You'll note that unlike the relationship between aliases and their aliasees, 
> the resolver and the ifunc have different types - the resolver takes no 
> parameters.
Okay.  I do wonder if there are attributes that we *should* take, but I agree 
that we probably shouldn't apply them by default; good catch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113352/new/

https://reviews.llvm.org/D113352

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5016
   llvm::Constant *Resolver =
-  GetOrCreateLLVMFunction(IFA->getResolver(), ResolverTy, GD,
+  GetOrCreateLLVMFunction(IFA->getResolver(), ResolverTy, {},
   /*ForVTable=*/false);

rjmccall wrote:
> Hmm, what was going on here?
The `emitIFuncDefinition` fucntion incorrectly passes the GlobalDecl of the 
IFunc itself to the call to GetOrCreateLLVMFunction for creating the resolver, 
which causes it to be created with a wrong attribute list, which fails 
`Verifier::visitFunction` with "Attribute after last parameter!". You'll note 
that unlike the relationship between aliases and their aliasees, the resolver 
and the ifunc have different types - the resolver takes no parameters.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113352/new/

https://reviews.llvm.org/D113352

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5016
   llvm::Constant *Resolver =
-  GetOrCreateLLVMFunction(IFA->getResolver(), ResolverTy, GD,
+  GetOrCreateLLVMFunction(IFA->getResolver(), ResolverTy, {},
   /*ForVTable=*/false);

Hmm, what was going on here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113352/new/

https://reviews.llvm.org/D113352

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein marked an inline comment as done.
ibookstein added a comment.

In D113352#3114009 , @dblaikie wrote:

> I guess CodeGenOpts::VerifyModule is on by default, but I'll admit that 
> surprises me - for API-built modules, I figured it was expected they were 
> valid by construction (ie: like an assertion - it's a bug in clang if the 
> resulting module isn't valid - so we wouldn't want to validate that invariant 
> on every user's compilation, etc) - so I'd expect we should have to opt-in to 
> verifying on output. (verifying on /input/ would be always done - because the 
> input is untrusted)

I agree that it is a bit surprising. It might make sense to change the default 
and make the clang llvm-lit tool substitutions opt-in.
However, it is a somewhat useful default when considered in its position as the 
entry into the codegen pipeline.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113352/new/

https://reviews.llvm.org/D113352

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 385361.
ibookstein edited the summary of this revision.
ibookstein added a comment.
Herald added subscribers: steven_wu, hiraditya.

- Fixed PR comment about documentation
- Amended clang/test/CodeGen/lto-newpm-pipeline.c to reflect change
- Fixed small bug in CodeGenModule::emitIFuncDefinition which made the 
clang/test/CodeGen/ifunc.c test fail verification


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113352/new/

https://reviews.llvm.org/D113352

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/lto-newpm-pipeline.c


Index: clang/test/CodeGen/lto-newpm-pipeline.c
===
--- clang/test/CodeGen/lto-newpm-pipeline.c
+++ clang/test/CodeGen/lto-newpm-pipeline.c
@@ -32,6 +32,8 @@
 // CHECK-FULL-O0-NEXT: Running pass: CoroCleanupPass
 // CHECK-FULL-O0-NEXT: Running pass: CanonicalizeAliasesPass
 // CHECK-FULL-O0-NEXT: Running pass: NameAnonGlobalPass
+// CHECK-FULL-O0-NEXT: Running pass: VerifierPass
+// CHECK-FULL-O0-NEXT: Running analysis: VerifierAnalysis
 // CHECK-FULL-O0-NEXT: Running pass: BitcodeWriterPass
 
 // CHECK-THIN-O0: Running pass: AlwaysInlinerPass
@@ -40,6 +42,8 @@
 // CHECK-THIN-O0: Running pass: CoroCleanupPass
 // CHECK-THIN-O0-NEXT: Running pass: CanonicalizeAliasesPass
 // CHECK-THIN-O0-NEXT: Running pass: NameAnonGlobalPass
+// CHECK-THIN-O0-NEXT: Running pass: VerifierPass
+// CHECK-THIN-O0-NEXT: Running analysis: VerifierAnalysis
 // CHECK-THIN-O0-NEXT: Running pass: ThinLTOBitcodeWriterPass
 
 // TODO: The LTO pre-link pipeline currently invokes
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5013,7 +5013,7 @@
   llvm::Type *DeclTy = getTypes().ConvertTypeForMem(D->getType());
   llvm::Type *ResolverTy = llvm::GlobalIFunc::getResolverFunctionType(DeclTy);
   llvm::Constant *Resolver =
-  GetOrCreateLLVMFunction(IFA->getResolver(), ResolverTy, GD,
+  GetOrCreateLLVMFunction(IFA->getResolver(), ResolverTy, {},
   /*ForVTable=*/false);
   llvm::GlobalIFunc *GIF =
   llvm::GlobalIFunc::create(DeclTy, 0, llvm::Function::ExternalLinkage,
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -487,6 +487,11 @@
   }
 }
 
+static bool actionRequiresCodeGen(BackendAction Action) {
+  return Action != Backend_EmitNothing && Action != Backend_EmitBC &&
+ Action != Backend_EmitLL;
+}
+
 static bool initTargetOptions(DiagnosticsEngine ,
   llvm::TargetOptions ,
   const CodeGenOptions ,
@@ -977,9 +982,7 @@
 
   setCommandLineOpts(CodeGenOpts);
 
-  bool UsesCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC &&
-  Action != Backend_EmitLL);
+  bool UsesCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(UsesCodeGen);
 
   if (UsesCodeGen && !TM)
@@ -1006,6 +1009,12 @@
 
   CreatePasses(PerModulePasses, PerFunctionPasses);
 
+  // Add a verifier pass if requested. We don't have to do this if the action
+  // requires code generation because there will already be a verifier pass in
+  // the code-generation pipeline.
+  if (!UsesCodeGen && CodeGenOpts.VerifyModule)
+PerModulePasses.add(createVerifierPass());
+
   legacy::PassManager CodeGenPasses;
   CodeGenPasses.add(
   createTargetTransformInfoWrapperPass(getTargetIRAnalysis()));
@@ -1425,6 +1434,13 @@
   MPM.addPass(ModuleMemProfilerPass());
 }
   }
+
+  // Add a verifier pass if requested. We don't have to do this if the action
+  // requires code generation because there will already be a verifier pass in
+  // the code-generation pipeline.
+  if (!actionRequiresCodeGen(Action) && CodeGenOpts.VerifyModule)
+MPM.addPass(VerifierPass());
+
   switch (Action) {
   case Backend_EmitBC:
 if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
@@ -1514,8 +1530,7 @@
   TimeRegion Region(CodeGenOpts.TimePasses ?  : nullptr);
   setCommandLineOpts(CodeGenOpts);
 
-  bool RequiresCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC && Action != 
Backend_EmitLL);
+  bool RequiresCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(RequiresCodeGen);
 
   if (RequiresCodeGen && !TM)


Index: clang/test/CodeGen/lto-newpm-pipeline.c
===
--- clang/test/CodeGen/lto-newpm-pipeline.c
+++ clang/test/CodeGen/lto-newpm-pipeline.c
@@ -32,6 +32,8 @@
 // CHECK-FULL-O0-NEXT: Running pass: CoroCleanupPass
 // CHECK-FULL-O0-NEXT: Running pass: CanonicalizeAliasesPass
 

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Seems like a good idea to me.  Minor request; otherwise, feel free to commit.




Comment at: clang/lib/CodeGen/BackendUtil.cpp:1438
+// Add it here so that it runs prior to the BC/LL emission pass
+MPM.addPass(VerifierPass());
+

This comment should be more like what you've got in your patch summary, e.g. 
"Add a verifier pass if requested.  We don't have to do this if the action 
requires code generation because there will already be a verifier pass in the 
code-generation pipeline."  (And it should go above the `if`.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113352/new/

https://reviews.llvm.org/D113352

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

I guess CodeGenOpts::VerifyModule is on by default, but I'll admit that 
surprises me - for API-built modules, I figured it was expected they were valid 
by construction (ie: like an assertion - it's a bug in clang if the resulting 
module isn't valid - so we wouldn't want to validate that invariant on every 
user's compilation, etc) - so I'd expect we should have to opt-in to verifying 
on output. (verifying on /input/ would be always done - because the input is 
untrusted)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113352/new/

https://reviews.llvm.org/D113352

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 385298.
ibookstein added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113352/new/

https://reviews.llvm.org/D113352

Files:
  clang/lib/CodeGen/BackendUtil.cpp


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -487,6 +487,11 @@
   }
 }
 
+static bool actionRequiresCodeGen(BackendAction Action) {
+  return Action != Backend_EmitNothing && Action != Backend_EmitBC &&
+ Action != Backend_EmitLL;
+}
+
 static bool initTargetOptions(DiagnosticsEngine ,
   llvm::TargetOptions ,
   const CodeGenOptions ,
@@ -977,9 +982,7 @@
 
   setCommandLineOpts(CodeGenOpts);
 
-  bool UsesCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC &&
-  Action != Backend_EmitLL);
+  bool UsesCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(UsesCodeGen);
 
   if (UsesCodeGen && !TM)
@@ -1006,6 +1009,10 @@
 
   CreatePasses(PerModulePasses, PerFunctionPasses);
 
+  if (!UsesCodeGen && CodeGenOpts.VerifyModule)
+// Add it here so that it runs prior to the BC/LL emission pass
+PerModulePasses.add(createVerifierPass());
+
   legacy::PassManager CodeGenPasses;
   CodeGenPasses.add(
   createTargetTransformInfoWrapperPass(getTargetIRAnalysis()));
@@ -1425,6 +1432,11 @@
   MPM.addPass(ModuleMemProfilerPass());
 }
   }
+
+  if (!actionRequiresCodeGen(Action) && CodeGenOpts.VerifyModule)
+// Add it here so that it runs prior to the BC/LL emission pass
+MPM.addPass(VerifierPass());
+
   switch (Action) {
   case Backend_EmitBC:
 if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
@@ -1514,8 +1526,7 @@
   TimeRegion Region(CodeGenOpts.TimePasses ?  : nullptr);
   setCommandLineOpts(CodeGenOpts);
 
-  bool RequiresCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC && Action != 
Backend_EmitLL);
+  bool RequiresCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(RequiresCodeGen);
 
   if (RequiresCodeGen && !TM)


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -487,6 +487,11 @@
   }
 }
 
+static bool actionRequiresCodeGen(BackendAction Action) {
+  return Action != Backend_EmitNothing && Action != Backend_EmitBC &&
+ Action != Backend_EmitLL;
+}
+
 static bool initTargetOptions(DiagnosticsEngine ,
   llvm::TargetOptions ,
   const CodeGenOptions ,
@@ -977,9 +982,7 @@
 
   setCommandLineOpts(CodeGenOpts);
 
-  bool UsesCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC &&
-  Action != Backend_EmitLL);
+  bool UsesCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(UsesCodeGen);
 
   if (UsesCodeGen && !TM)
@@ -1006,6 +1009,10 @@
 
   CreatePasses(PerModulePasses, PerFunctionPasses);
 
+  if (!UsesCodeGen && CodeGenOpts.VerifyModule)
+// Add it here so that it runs prior to the BC/LL emission pass
+PerModulePasses.add(createVerifierPass());
+
   legacy::PassManager CodeGenPasses;
   CodeGenPasses.add(
   createTargetTransformInfoWrapperPass(getTargetIRAnalysis()));
@@ -1425,6 +1432,11 @@
   MPM.addPass(ModuleMemProfilerPass());
 }
   }
+
+  if (!actionRequiresCodeGen(Action) && CodeGenOpts.VerifyModule)
+// Add it here so that it runs prior to the BC/LL emission pass
+MPM.addPass(VerifierPass());
+
   switch (Action) {
   case Backend_EmitBC:
 if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
@@ -1514,8 +1526,7 @@
   TimeRegion Region(CodeGenOpts.TimePasses ?  : nullptr);
   setCommandLineOpts(CodeGenOpts);
 
-  bool RequiresCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC && Action != Backend_EmitLL);
+  bool RequiresCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(RequiresCodeGen);
 
   if (RequiresCodeGen && !TM)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein created this revision.
ibookstein added reviewers: dblaikie, rjmccall.
Herald added a subscriber: ormris.
ibookstein requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Previously, the Backend_Emit{Nothing,BC,LL} modes did
not run the LLVM verifier since it is usually added via
the TargetMachine::addPassesToEmitFile method according
to the DisableVerify parameter. This is called from
EmitAssemblyHelper::AddEmitPasses, which is only relevant
for BackendAction-s that require CodeGen.

Note:

- In these particular situations the verifier is added to the optimization 
pipeline rather than the codegen pipeline so that it runs prior to the BC/LL 
emission pass.
- This change applies to both the old and the new PMs.
- Because the clang tests use -emit-llvm ubiquitously, this change will enable 
the verifier for them.

Signed-off-by: Itay Bookstein 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113352

Files:
  clang/lib/CodeGen/BackendUtil.cpp


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -487,6 +487,12 @@
   }
 }
 
+static bool actionRequiresCodeGen(BackendAction Action) {
+  return Action != Backend_EmitNothing &&
+ Action != Backend_EmitBC &&
+ Action != Backend_EmitLL;
+}
+
 static bool initTargetOptions(DiagnosticsEngine ,
   llvm::TargetOptions ,
   const CodeGenOptions ,
@@ -977,9 +983,7 @@
 
   setCommandLineOpts(CodeGenOpts);
 
-  bool UsesCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC &&
-  Action != Backend_EmitLL);
+  bool UsesCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(UsesCodeGen);
 
   if (UsesCodeGen && !TM)
@@ -1006,6 +1010,10 @@
 
   CreatePasses(PerModulePasses, PerFunctionPasses);
 
+  if (!UsesCodeGen && CodeGenOpts.VerifyModule)
+// Add it here so that it runs prior to the BC/LL emission pass
+PerModulePasses.add(createVerifierPass());
+
   legacy::PassManager CodeGenPasses;
   CodeGenPasses.add(
   createTargetTransformInfoWrapperPass(getTargetIRAnalysis()));
@@ -1425,6 +1433,11 @@
   MPM.addPass(ModuleMemProfilerPass());
 }
   }
+
+  if (!actionRequiresCodeGen(Action) && CodeGenOpts.VerifyModule)
+// Add it here so that it runs prior to the BC/LL emission pass
+MPM.addPass(VerifierPass());
+
   switch (Action) {
   case Backend_EmitBC:
 if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
@@ -1514,8 +1527,7 @@
   TimeRegion Region(CodeGenOpts.TimePasses ?  : nullptr);
   setCommandLineOpts(CodeGenOpts);
 
-  bool RequiresCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC && Action != 
Backend_EmitLL);
+  bool RequiresCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(RequiresCodeGen);
 
   if (RequiresCodeGen && !TM)


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -487,6 +487,12 @@
   }
 }
 
+static bool actionRequiresCodeGen(BackendAction Action) {
+  return Action != Backend_EmitNothing &&
+ Action != Backend_EmitBC &&
+ Action != Backend_EmitLL;
+}
+
 static bool initTargetOptions(DiagnosticsEngine ,
   llvm::TargetOptions ,
   const CodeGenOptions ,
@@ -977,9 +983,7 @@
 
   setCommandLineOpts(CodeGenOpts);
 
-  bool UsesCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC &&
-  Action != Backend_EmitLL);
+  bool UsesCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(UsesCodeGen);
 
   if (UsesCodeGen && !TM)
@@ -1006,6 +1010,10 @@
 
   CreatePasses(PerModulePasses, PerFunctionPasses);
 
+  if (!UsesCodeGen && CodeGenOpts.VerifyModule)
+// Add it here so that it runs prior to the BC/LL emission pass
+PerModulePasses.add(createVerifierPass());
+
   legacy::PassManager CodeGenPasses;
   CodeGenPasses.add(
   createTargetTransformInfoWrapperPass(getTargetIRAnalysis()));
@@ -1425,6 +1433,11 @@
   MPM.addPass(ModuleMemProfilerPass());
 }
   }
+
+  if (!actionRequiresCodeGen(Action) && CodeGenOpts.VerifyModule)
+// Add it here so that it runs prior to the BC/LL emission pass
+MPM.addPass(VerifierPass());
+
   switch (Action) {
   case Backend_EmitBC:
 if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
@@ -1514,8 +1527,7 @@
   TimeRegion Region(CodeGenOpts.TimePasses ?  : nullptr);
   setCommandLineOpts(CodeGenOpts);
 
-  bool RequiresCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC && Action != Backend_EmitLL);
+  bool