[GlobalISel][RFC] New verifier stages

[Hal Finkel via llvm-commits]

----- Original Message -----
> From: "Quentin Colombet" <qcolombet at apple.com>
> To: "llvm-commits" <llvm-commits at lists.llvm.org>
> Cc: "Chandler Carruth" <chandlerc at gmail.com>, "Eric Christopher" <echristo at gmail.com>, "Hal Finkel"
> <hfinkel at anl.gov>, "Matthias Braun" <matze at braunis.de>, "Chris Lattner" <clattner at apple.com>
> Sent: Friday, December 11, 2015 7:44:17 PM
> Subject: [GlobalISel][RFC] New verifier stages
> 
> Hi,
> 
> 
> I would like your opinion on the best option to check the
> pre/post-conditions for the passes involved in GlobalISel.
> My preference goes with the “Attach the Information to The
> MachineFunction” but I would like to have your feedbacks.
> 
> 
> ** Context **
> 
> In GlobalISel, the selection process will be split into separate
> passes.
> As a reminder, this is the suggested pipeline:
> LLVM IR -> IRTranslator -> (G)MI -> Legalizer -> RegBankSelect ->
> Select -> MI
> 
> Each pass produces MachineInstr with some properties and has some
> conditions on which it operates.
> For instance, after the Legalizer everything is legal and before
> RegBankSelect everything must be legal.
> 
> Targets can inject passes between the generic passes, i.e., they may
> inadvertently break the assumptions made by the generic passes.
> We need to have a simple way to catch that.
> 
> 
> ** Problem **
> 
> The MachineVerifier has not way to know when it is running. For
> instance, it is impossible to check that there are no virtual
> registers left after register allocation because it does not know
> that it is run after register allocation. In such bad situation, we
> would probably be hitting a crash/assert in the MC layer complaining
> about a register it does not know how to encode.
> 
> Right now, things run optionally in the MachineVerifier based on the
> getAnalysisIfAvailable feature. This does not work for anything that
> is not carried by an analysis like where we are in the pipeline.
> 
> 
> ** Options **
> 
> * Embedded a Verifier in the Generic Passes *
> 
> Add pre/post-verify method to the generic passes that would be called
> at the beginning/end of the runOnXXX method.
> 
> Pros:
> - The assumptions are checked at the very point where they are
> required. E.g., a target pass could insert illegal code that could
> be cleared by another target pass between the legalizer and
> regbankselect.
> 
> Cons:
> - Unable for the target to use these verifiers.
> - No sharing, a priori, between the verifiers of two passes in a row
> like Legalizer and RegBankSelect.
> 
> 
> * New Specific Verifiers *
> 
> Add specific machine verifiers in the pipeline.
> 
> Pros:
> - We check for what we want.
> - Code reuse.
> 
> Cons:
> - We lose, or have to patch, all the hooks to register a machine
> verifier after/before each pass.
> - We need to add calls to the proper verifier at the proper spots.
> 
> 
> * Convey the Information In the Pass Manager? *
> 
> Have the PassManager telling the passes where we are in the pipeline.
> 
> When discussing with Ahmed, he suggested that the pass manager may
> convey this kind of information or that it may be something we want
> to be able to do.
> @Chandler, what do you think could be done here? Does it even make
> sense?
> I was thinking like defining a stage or something that the passes
> could get and that would be defined when we build the pipeline, but
> that the pass manager would actually switch after it runs the
> related pass.
> E.g., addPass(Select); setStage(Stage::PostSelection);
> 
> The pass manager would remember to switch the stage to PostSelection
> right after it runs Select.
> 
> 
> * Attach the Information to The MachineFunction *
> 
> Add an attribute in MachineFunction holding the current stage and
> have the MachineVerifier to act accordingly.
> 
> This is pretty much similar to what we do with SDISel where we have
> booleans that tell if we are before legalization, etc.
> This is the most straight forward approach and it has my preference.
> 
> Pros:
> - One MachineVerifier for everything.
> - Build on top of the existing infrastructure.
> 
> Cons:
> - Target specific passes cannot temporarily break the assumptions
> between two target specific passes (I would almost put that as a
> pro, but who knows :)).
> - More information in MachineFunction.
> - Not a general solution. E.g., maybe an analysis pass wants to know
> if it runs before or after PEI to collect slightly different
> information.

My initial reaction is to favor this solution. It is similar to what we already do with isSSA() in MachineRegisterInfo.

 -Hal

> 
> * Other? *
> 
> <Insert your solution here :)>
> 
> What do people think?
> 
> Thanks,
> -Quentin

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory