[GlobalISel][RFC] New verifier stages

Hi Medhi,

Hi,

Side note: shouldn’t RFCs rather go to llvm-dev?

You’re right!
I could have sworn I have sent it to llvm-dev!
Moving it now.

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.

I find this to be the most elegant: it is stateless and decoupled from the pass manager.

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.

I don’t understand the Cons, maybe because I don’t see the implementation you have in mind.

Let me illustrate a rough sketch:

class MyMachinePass {

public:
// Returns false on error
bool preCheckIR(MachineFunction &F) {
MachineVerifier V;
V.setSSA(true);
V.allowsGeneric(false);
return V.verify(F);
}

// Returns false on error
void postCheckIR(MachineFunction &F) {
MachineVerifier V;
V.setSSA(true);
V.allowsGeneric(false);
V.verify(F);
}

}

This way:

  • the PassManager does not need to know about any “stage” or pass specific stuff, it calls these hooks like it already does for other features.
  • the Passes can parameterize the verifier
  • the Target Passes can reuse the verifier, maybe extend it or parameterize differently

How?
Sorry, I do not see that.

  • Code sharing is not an issue.

Again how?

For Instance, the postCheckIR for the Legalizer will be the same as the preCheckIR for RegBankSelect.
Moreover, any passes run in between may want to check against those conditions.

Could you clarify?

Thanks,
-Quentin

Talked with Medhi off-line.

Giving a summary for the record.

Hi Medhi,

Hi,

Side note: shouldn’t RFCs rather go to llvm-dev?

You’re right!
I could have sworn I have sent it to llvm-dev!
Moving it now.

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.

I find this to be the most elegant: it is stateless and decoupled from the pass manager.

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.

I don’t understand the Cons, maybe because I don’t see the implementation you have in mind.

Let me illustrate a rough sketch:

class MyMachinePass {

public:
// Returns false on error
bool preCheckIR(MachineFunction &F) {
MachineVerifier V;
V.setSSA(true);
V.allowsGeneric(false);
return V.verify(F);
}

// Returns false on error
void postCheckIR(MachineFunction &F) {
MachineVerifier V;
V.setSSA(true);
V.allowsGeneric(false);
V.verify(F);
}

}

This way:

  • the PassManager does not need to know about any “stage” or pass specific stuff, it calls these hooks like it already does for other features.
  • the Passes can parameterize the verifier
  • the Target Passes can reuse the verifier, maybe extend it or parameterize differently

How?
Sorry, I do not see that.

This does not work because a pass does not necessary knows where it is run in the pipeline. E.g., we could run MachineCSE anywhere in the pipeline and thus, those pre/post IR checks cannot be decided per pass.

One more point in favor to the state in the MachineFunction, is that it will get serialized with the MachineFunction. I.e., we would be able to complain if we give a function is in a different state that what is expected when writing tests.

Thanks Mehdi for pointing that out!

Cheers,
-Quentin

Just to clarify even more for everyone (because I didn’t get the problem before Quentin explained it to me offline): MachineCSE (or other generic pass) could use a verifier to check its own pre/post condition which are at the "generic IR” level, and that should work anywhere.
But if we are late in the pipeline, we’d like the verifier to understand and enforce Target-specific constraints. In order to do so, there is some state that needs to be stored somewhere (and for the reason Quentin stated below, the MachineFunction can be a good candidate).

Thanks Quentin!