[Instrumentation] Support verifying machine function #90931
Conversation
paperchalice
changed the title
paperchalice
force-pushed
the
verify-instrumentation
branch
from
5db6744 to
4c847d2
Compare
| @@ -115,7 +115,7 @@ int llvm::compileModuleWithNewPM( | |||
| MachineModuleInfo MMI(&LLVMTM); | |||
|
|
|||
| PassInstrumentationCallbacks PIC; | |||
| StandardInstrumentations SI(Context, Opt.DebugPM); | |||
| StandardInstrumentations SI(Context, Opt.DebugPM, !NoVerify); | |||
There was a problem hiding this comment.
Command line option verify-machineinstrs is overlapped with --disable-verify when we use new pass manager.
|
|
||
| // TODO: Use complete MachineVerifierPass. | ||
| if (auto *MF = unwrapIR<MachineFunction>(IR)) { | ||
| dbgs() << "Verifying machine function " << MF->getName() << '\n'; |
There was a problem hiding this comment.
logging should be under DebugLogging
| // TODO: Use complete MachineVerifierPass. | ||
| if (auto *MF = unwrapIR<MachineFunction>(IR)) { | ||
| dbgs() << "Verifying machine function " << MF->getName() << '\n'; | ||
| verifyMachineFunction("", *MF); |
There was a problem hiding this comment.
should we add something for Banner, like the "Broke module found after pass ..." above? and we have a pass that breaks MIR to test this IIRC?
There was a problem hiding this comment.
MIRs are verified immediately after parsing and there is no trivial pass that convert valid mir to invalid mir.
There was a problem hiding this comment.
ah I was thinking of RequireAllMachineFunctionPropertiesPass. could you add something like that pass which creates invalid MIR, then use that for testing?
There was a problem hiding this comment.
Extended TriggerVerifierErrorPass now.
| @@ -0,0 +1,10 @@ | |||
| # RUN: llc -mtriple=x86_64-pc-linux-gnu -x mir -passes=no-op-machine-function -filetype=null < %s 2>&1 | FileCheck %s | |||
There was a problem hiding this comment.
this needs to enable DebugLogging right?
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
paperchalice
force-pushed
the
verify-instrumentation
branch
from
453676a to
0260bdb
Compare
| PreservedAnalyses run(MachineFunction &MF, MachineFunctionAnalysisManager &) { | ||
| // Intentionally create a virtual register and set NoVRegs property. | ||
| auto &MRI = MF.getRegInfo(); | ||
| MRI.createGenericVirtualRegister(LLT::scalar(8)); |
There was a problem hiding this comment.
To be safe, it would be best to introduce an actual instruction with the register. IIRC we permit virtual registers that have no uses in the MIR
paperchalice
force-pushed
the
verify-instrumentation
branch
from
0260bdb to
237b57f
Compare
| # RUN: not --crash llc -mtriple=x86_64-pc-linux-gnu -debug-pass-manager -passes='module(function(machine-function(trigger-verifier-error)))' -filetype=null %s 2>&1 | FileCheck %s | ||
|
|
||
| # CHECK: Verifying machine function f | ||
| # CHECK: Broken machine function found after pass |
There was a problem hiding this comment.
nit: CHECK: ... after pass "TriggerVerifierErrorPass"
We need it to test isel related passes. Currently `verifyMachineFunction` is incomplete (no LiveIntervals support), but is enough for testing isel pass, will migrate to complete `MachineVerifierPass` in future.
We need it to test isel related passes. Currently
verifyMachineFunctionis incomplete (no LiveIntervals support), but is enough for testing isel pass, will migrate to completeMachineVerifierPassin future.