Upper case vs lower case in printed and parsed MIR

I am confused about the rules for when upper and lower case letters should be used in MIR.

As an example our downstream target has upper case letters in its sub-register indices and as a result we cannot import exported MIR without manually ‘lower casing’ it first which is obviously rather annoying.

Looking in https://llvm.org/docs/MIRLangRef.html it is stated that instruction names are case sensitive.

For register names it appear that they are lower cased before printing (see printReg in TargetRegisterInfo.cpp) and to match the definitions are also lower cased before loaded into the parser (see PerTargetMIParsingState::initNames2Regs in MIParser.cpp). For sub-register index names the latter happens but they are currently printed with their original casing witch leads to our problem.

What is the right solution here, should they be lower cased when printing as well (as I tried to do in https://reviews.llvm.org/D60311)?

To me it seems that preserving the original casing from the .td file would be the most correct thing to do but then it would be inconsistent with e.g. register names and would only add to the confusion it seems.

Thanks
Markus

From: llvm-dev <llvm-dev-bounces@lists.llvm.org> On Behalf Of Markus Lavin
via llvm-dev
Sent: den 11 april 2019 19:40
To: llvm-dev@lists.llvm.org
Subject: [llvm-dev] Upper case vs lower case in printed and parsed MIR

I am confused about the rules for when upper and lower case letters should
be used in MIR.

As an example our downstream target has upper case letters in its sub-
register indices and as a result we cannot import exported MIR without
manually 'lower casing' it first which is obviously rather annoying.

Looking in https://llvm.org/docs/MIRLangRef.html it is stated that
instruction names are case sensitive.

For register names it appear that they are lower cased before printing (see
printReg in TargetRegisterInfo.cpp) and to match the definitions are also
lower cased before loaded into the parser
(see PerTargetMIParsingState::initNames2Regs in MIParser.cpp). For sub-
register index names the latter happens but they are currently printed with
their original casing witch leads to our problem.

Do we know if making register names case sensitive would be a big churn
(e.g. in .mir test cases)? I assume it would have quite big impact.

What is the right solution here, should they be lower cased when printing
as well (as I tried to do in https://reviews.llvm.org/D60311)?

To me it seems that preserving the original casing from the .td file would
be the most correct thing to do but then it would be inconsistent with e.g.
register names and would only add to the confusion it seems.

Handling register names and subregister names consistently actually sounds
reasonable. Even though my first feeling was that it would be nice to print
the subregister names with the same casing as used in the code.

There are however more strings so it is hard to know where to draw the line
when it comes to forcing lower case in the API, if instruction names are
still should be case sensitive. Should for example register class names
follow the same rule as for registers?

If we want to force lower case (e.g. for register/subregister names), then
I think we want to avoid doing the lower casing at runtime and instead
tablegen should put the lower case names in the tables already from the start.

There is ofcourse the option of making the parsing case-insensitive (for
register names, subregister names). I guess that would make least churn,
since things would be backwards compatible with existing test cases (and
printouts could be handled just like today, even if it isn't consistent).

@Markus: We could even rename things in the RegisterInfo.td file for our OOT
target to use lower case names (and by that hide the problems we currently see).
But that would not help the community. So it would be nice to hear what
others have to say about this.

Today the MIR parser expects lower case subregister names only.
It then matches the parsed name with an (at runtime) lower casing
of the subregister names stored in tablegen'd tables.

Since there has been no more comments on these questions from
Markus I suggest that D60311 should be abandoned, and then we should
make a solution (focusing on subregister names) where the MIR parser
should be case sensitive when parsing subregister names.

(If we run into trouble with an unexpected amount of churn in any MIR
testcases we could go for making the parser case insensitive.)

Thus, the MIR parser won't be fully backwards compatible. And for example
downstream repo maintainers (like myself) would need to update MIR
tests to use the correct case in subregister names.

/Björn

Abandoned D60311 and opened D61499 in favour of case sensitivity.

-Markus