Page MenuHomePhabricator

[RFC] CodeGen: Print/parse LLTs in MachineMemOperands
ClosedPublic

Authored by arsenm on May 20 2021, 1:35 PM.

Details

Summary

This will currently accept the old number of bytes syntax, and convert
it to a scalar. This should be removed in the near future (I think I
converted all of the tests already, but likely missed a few).

Not sure what the exact syntax and policy should be. We can continue
printing the number of bytes for non-generic instructions to avoid
test churn and only allow non-scalar types for generic instructions.

This will currently print the LLT in parentheses, but accept parsing
the existing integers and implicitly converting to scalar. The
parentheses are a bit ugly, but the parser logic seems unable to deal
without either parentheses or some keyword to indicate the start of a
type.

Diff Detail

Event Timeline

arsenm created this revision.May 20 2021, 1:35 PM
arsenm requested review of this revision.May 20 2021, 1:35 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 20 2021, 1:35 PM
Herald added a subscriber: wdng. · View Herald Transcript

In general I think the syntax looks fine. Not sure I get how hard it is to get rid of the parenthesis, but if we end up changing many tests, we should probably do it only once.

Suggestion: maybe add a flag to facilitate the test transition for downstream users? (for CHECK lines, assuming there would be some tests that check for things like load 4 instead of load (s32))

arsenm updated this revision to Diff 350734.Jun 8 2021, 4:20 PM
arsenm edited the summary of this revision. (Show Details)

I completed the test updates, however the diff is at least 3x larger than the phabricator upload maximum size.

Should be able to drop the legacy parsing of the integer size soon in a follow on change

arsenm added a comment.Jun 8 2021, 4:21 PM

In general I think the syntax looks fine. Not sure I get how hard it is to get rid of the parenthesis, but if we end up changing many tests, we should probably do it only once.

Suggestion: maybe add a flag to facilitate the test transition for downstream users? (for CHECK lines, assuming there would be some tests that check for things like load 4 instead of load (s32))

I think this will just contribute to the neverending accumulation of compatibility junk to avoid test updates. At least for globalisel tests, more manual intervention is needed to fixup cases where the scalar is inappropriate. We'll need some memory type verifiers for G_* instructions

arsenm retitled this revision from [RFC] [WIP] CodeGen: Print/parse LLTs in MachineMemOperands to [RFC] CodeGen: Print/parse LLTs in MachineMemOperands.Jun 8 2021, 4:22 PM
arsenm updated this revision to Diff 350758.Jun 8 2021, 6:58 PM

Fix non power of 2 default alignment for weird types

Can't we do a token lookahead or something to determine if we need to parse a uint or an LLT?

Can't we do a token lookahead or something to determine if we need to parse a uint or an LLT?

This is ready to drop parsing the uint path entirely (although I'll do that as a follow on change)

aemerson accepted this revision.Jun 30 2021, 11:30 AM
This revision is now accepted and ready to land.Jun 30 2021, 11:30 AM