llvm dev - Dec 2015 - [GlobalISel][RFC] New verifier stages

Hi Medhi,
> On Dec 11, 2015, at 6:18 PM, Mehdi Amini <mehdi.amini at apple.com>
wrote:
> 
> 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.
> 
>> On Dec 11, 2015, at 5:44 PM, Quentin Colombet via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>> 
>> 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
> 
> — 
> Mehdi
> 
> 
>> 
>> 
>> * 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.
>> 
>> * Other? *
>> 
>> <Insert your solution here :)>
>> 
>> What do people think?
>> 
>> Thanks,
>> -Quentin
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org <mailto:llvm-commits at
lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151214/5d041c26/attachment-0001.html>

Talked with Medhi off-line.

Giving a summary for the record.
> On Dec 14, 2015, at 9:27 AM, Quentin Colombet via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Hi Medhi,
> 
>> On Dec 11, 2015, at 6:18 PM, Mehdi Amini <mehdi.amini at apple.com
<mailto:mehdi.amini at apple.com>> wrote:
>> 
>> 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.
> 
>> 
>>> On Dec 11, 2015, at 5:44 PM, Quentin Colombet via llvm-commits
<llvm-commits at lists.llvm.org <mailto:llvm-commits at
lists.llvm.org>> wrote:
>>> 
>>> 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
> 
>> - 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
>> 
>> — 
>> Mehdi
>> 
>> 
>>> 
>>> 
>>> * 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.
>>> 
>>> * Other? *
>>> 
>>> <Insert your solution here :)>
>>> 
>>> What do people think?
>>> 
>>> Thanks,
>>> -Quentin
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org <mailto:llvm-commits at
lists.llvm.org>
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151214/3b0265d8/attachment.html>

> On Dec 14, 2015, at 10:29 AM, Quentin Colombet <qcolombet at
apple.com> wrote:
> 
> Talked with Medhi off-line.
> 
> Giving a summary for the record.
> 
>> On Dec 14, 2015, at 9:27 AM, Quentin Colombet via llvm-dev <llvm-dev
at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
>> 
>> Hi Medhi,
>> 
>>> On Dec 11, 2015, at 6:18 PM, Mehdi Amini <mehdi.amini at
apple.com <mailto:mehdi.amini at apple.com>> wrote:
>>> 
>>> 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.
>> 
>>> 
>>>> On Dec 11, 2015, at 5:44 PM, Quentin Colombet via llvm-commits
<llvm-commits at lists.llvm.org <mailto:llvm-commits at
lists.llvm.org>> wrote:
>>>> 
>>>> 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.
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!

— 
Mehdi
> 
> 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
> 
>> 
>>> - 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
>>> 
>>> — 
>>> Mehdi
>>> 
>>> 
>>>> 
>>>> 
>>>> * 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.
>>>> 
>>>> * Other? *
>>>> 
>>>> <Insert your solution here :)>
>>>> 
>>>> What do people think?
>>>> 
>>>> Thanks,
>>>> -Quentin
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org <mailto:llvm-commits at
lists.llvm.org>
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151214/f0dd2f46/attachment-0001.html>