[PATCH] D111397: Add new MachineFunction property FailsVerification

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/MIRYamlMapping.h:726
     YamlIO.mapOptional("hasWinCFI", MF.HasWinCFI, false);
+    YamlIO.mapOptional("failsVerification", MF.FailsVerification, false);
     YamlIO.mapOptional("registers", MF.VirtualRegisters,
----------------
foad wrote:
> arsenm wrote:
> > This should not need explicit serialization. The verifier runs after mir parsing anyway. 
> > 
> > It should also be illegal to clear this after it’s set 
> > This should not need explicit serialization.
> 
> Why? It means "this function has known problems that I haven't fixed yet so please don't run the verifier on it".
> 
> > It should also be illegal to clear this after it’s set
> 
> Why?
Oh, I misread the intent of this change. I assumed this was solving a problem I've run into where a pass introduces a compilation failure, and as a result ends up in some invalid state where the verifier crashes. For example this happens when the register allocator fails and emits an error, but the compilation still needs to continue.

I don't love the idea of allowing functions to fail the verifier, and then be fixed up later. Once the verifier fails a function should be dead. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111397