[TableGen][M68K] (Patch 1/8) Utilities for complex instruction addressing modes: CodeBeads and logical operand helper functions

[myhsu]

Summary

This patch adds two components: The CodeBeads TG backend and the support for InstrInfoEmitter TG backend to emit information about logical operands.

CodeBeads are annotations for instructions to express _non-trivial amount_ of addressing modes in an easier way. Many instructions of M68K, like many other CISC architectures, can be used with variety of addressing modes. For example, JSR (i.e. function call) can be used with four different addressing modes. Without this CodeBeads utility to explicitly embed these information into instructions' TG definitions, it would be more difficult for MC to encode the instructions (e.g. need to write some sort of pattern matching code to figure out the addressing mode).

Another special property of CISC ISA is that a single instruction operand (called "logical operand" here) might consist of multiple llvm::MachineOperands. Thus this patch enables IntrInfoEmitter TG backend to (optionally) generate helper functions for logical operands.
More specifically, the llvm::<target NS>::getLogicalOperandSize to get the number of llvm::MachineOperand on a specific logical operand; llvm::<target NS>::getLogicalOperandIdx to get the llvm::MachineOperand index for a specific logical operand.

[Paul-C-Anagnostopoulos]

Everything looks good except for the Clang tidies that need attention. A couple of them pertain to the skeleton backend file, so I will fix those in that file, too.

Let's wait a couple of days to see if anyone else has comments.

[jrtc27]

llvm/utils/TableGen/CodeBeadsGen.cpp

10

This needs to be filled in before landing.

llvm/utils/TableGen/TableGen.cpp

28

I don't understand what order these are in, so why here rather than at the end of the enum?

[myhsu]

• Fix formatting issues
• Add documentation for CodeBeads in the header comment

[rengolin]

There are plenty of lint checks that need addressing. Some are false positives and those are obvious cases, but the rest should be followed.

You must run clang-format on new files (entirely) and on localised changes of existing files (many editors have the option to select lines and format).

llvm/utils/TableGen/CodeBeadsGen.cpp

6

The license has changed to the Apache license (+LLVM exception).

See LICENSE.TXT.

You'll need to change that in all your files.

[rengolin]

Adding @craig.topper as the owner of the x86 back-end, to check the implementation of the CodeBeads concept. I have mainly worked with RISC back-ends, so it's not my area. Craig, please feel free to add whoever you think is best to review this.

[myhsu]

In D88385#2305465, @rengolin wrote:

There are plenty of lint checks that need addressing. Some are false positives and those are obvious cases, but the rest should be followed.

You must run clang-format on new files (entirely) and on localised changes of existing files (many editors have the option to select lines and format).

Interesting...the patch I updated yesterday had gone through clang-format. And I didn't see any lint warning here on Phabricator. Maybe I should run again.

[Paul-C-Anagnostopoulos]

Don't forget to change to the current license.

[myhsu]

Update license

[rengolin]

In D88385#2307461, @myhsu wrote:

Interesting...the patch I updated yesterday had gone through clang-format. And I didn't see any lint warning here on Phabricator. Maybe I should run again.

Did you check that you were using the latest llvm configuration?

[myhsu]

Decouple getGenInstrBeads from <Target>MCCodeEmitter and put it as a static function in llvm::<Target namespace> with new name getMCInstrBeads

[myhsu]

In D88385#2307853, @rengolin wrote:

In D88385#2307461, @myhsu wrote:

Interesting...the patch I updated yesterday had gone through clang-format. And I didn't see any lint warning here on Phabricator. Maybe I should run again.

Did you check that you were using the latest llvm configuration?

I made sure clang-format was fetching the the right config file. Would you mind pointing out some of the places that you think ill-format? thank you!

[craig.topper]

llvm/utils/TableGen/CMakeLists.txt

59

This list was nearly in alphabetical order. Can you put this file in the correct place.

llvm/utils/TableGen/CodeBeadsGen.cpp

51

Where do these numbers come from? Are they specific to 68K?

86

Remove commented out code?

93

You can use auto here

98

I think you can use Twine(i) here instead of std::to_string(i).

llvm/utils/TableGen/InstrInfoEmitter.cpp

475

std::max?

479

Is the find and if necessary? Won't insert avoid overwriting value if its already in the map?

483

I think you can use (Namespace + "::" + Inst->TheDef->getName()).str(). That will create a Twine for the pieces and the convert it to a std::string at the end. This should be slightly more efficient than concatenating std::strings.

527

I think we should just use a range for loop here. llvm::for_each is discouraged here https://llvm.org/docs/CodingStandards.html#use-range-based-for-loops-wherever-possible

570

Same comment as earlier about using a Twine here.

579

I think insert is sufficient.

[craig.topper]

llvm/utils/TableGen/InstrInfoEmitter.cpp

506

Probably don't need to bother avoiding trailing commas in the generated file. The C++ parsing rules allow them

576

std::max?

627

range for

[myhsu]

Addressed feedbacks, mostly formatting issues and minor coding improvements

llvm/utils/TableGen/CodeBeadsGen.cpp

51

no really...this number depends on maximum bit length among all code beads. As the TODO comment on line 55 suggested we should have a way to evaluate this dynamically. Also the for loop on line 84 need this number to reverse the byte order

[Paul-C-Anagnostopoulos]

A minor suggestion: Use the new true/false literals in the TableGen files. I believe it makes the code easier to read.

[RKSimon]

llvm/utils/TableGen/CodeBeadsGen.cpp

51

Please add a comment explaining these magic numbers.

llvm/utils/TableGen/InstrInfoEmitter.cpp

498

const auto &Row

516

const auto &Instrs

590

const auto &Row

614

const auto &Insts

[Paul-C-Anagnostopoulos]

llvm/include/llvm/Target/Target.td

645

Before you push this, Target.td will use 'true' and 'false'.

llvm/utils/TableGen/CodeBeadsGen.cpp

34

It's conventional to call the output stream OS.

[myhsu]

llvm/utils/TableGen/CodeBeadsGen.cpp

51

Eventually I decided to just fix that TODO so the next revision will not have that magic number anymore

[myhsu]

• Rebase to upstream master
• Addressed feedbacks
• Update to use the latest TG syntax

[RKSimon]

Does anyone have any more comments? Otherwise I think we can provisionally accept this.

@rengolin I think you wanted to see the entire patch series to be accepted before anything gets committed?

[rengolin]

In D88385#2442226, @RKSimon wrote:

Does anyone have any more comments? Otherwise I think we can provisionally accept this.

LGTM too, thanks!

@jrtc27, you have marked as blocked. Have all your issues been resolved?

@rengolin I think you wanted to see the entire patch series to be accepted before anything gets committed?

Definitely. We need to make sure the whole series is sound, then wait a bit to make sure no one else has any concern, then all patches can be committed one after another.

Feel free to mark any patch of the series as accepted, so that we can speed up the convergence.

[RKSimon]

LGTM

[RKSimon]

@jrtc27 Are you happy to approve this now? You're currently blocking the patch.

[jrtc27]

llvm/include/llvm/Target/Target.td

642

would -> will

643–644

Reading this I don't actually understand what it does. How can I map a type to an operand of an MI?

llvm/utils/TableGen/CodeBeadsGen.cpp

9

I don't understand where the name "code bead" comes from. There's no reference to it in the GCC source, nor on the internet in general that I can obviously find, so it sounds to me like something you've invented yourself. Can we please stick to more standard terminology for things? Bead is meaningless.

96

uint8_t?

119

Array decay should render the cast unnecessary?

llvm/utils/TableGen/InstrInfoEmitter.cpp

483

Still not done; the std::string construction for Namespace is not necessary.

500–501

[MaskRay]

llvm/include/llvm/Target/Target.td

642

https://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments says "don't use example - "

Many older functions do not conform to the standard but newer functions should.

[RKSimon]

llvm/utils/TableGen/CodeBeadsGen.cpp

9

Alternatively a more expansive description of code beads should be OK - e.g. are they just for helping emit CISC variable length bit encodings?

[myhsu]

• [NFC] Addressed feedbacks

[jrtc27]

I still don't understand what problem code beads are trying to solve that isn't already solved by existing backends like X86. Why can't you just assign operands to instruction encoding bits like a normal backend?

[craig.topper]

llvm/utils/TableGen/CodeBeadsGen.cpp

18

speicifc -> specific

[myhsu]

In D88385#2475335, @jrtc27 wrote:

I still don't understand what problem code beads are trying to solve that isn't already solved by existing backends like X86. Why can't you just assign operands to instruction encoding bits like a normal backend?

Currently X86 - probably the only Target that shares a similar problem - uses MCInstrDesc::TSFlags to carry complex encoding info (maybe @craig.topper can shed some lights here).

TSFlags is only 64-bits wide. But some of the M68k instructions need 24 bytes to carry its encoding info. Frankly speaking, this is simply because we didn't come up with a optimal way to fit every info into 64 bits when we first created this code base long time ago (I don't think M68k has much more complex instruction formats than X86).

I'm happy to find if there is a way to fit all encoding info into TSFlags - and that will bring major code changes to this patch series for sure - if everyone here think this is the approach we should take.

[RKSimon]

Do you have any idea how much work would be necessary for the TSFlags refactor?

@rengolin @craig.topper any thoughts?

[Paul-C-Anagnostopoulos]

If it isn't possible to encode the information in TSFlags, another possibility is to add a pointer to the MCInstrDesc class that points to some kind of auxiliary class/struct. It would be unfortunate to increase the size of MCInstrDesc, though. I have been playing with the idea of changing the three pointer members to indexes in order to save memory, but I'm not yet convinced it's worth the effort. There are about 67,000 instances across the targets.

[myhsu]

In D88385#2476167, @RKSimon wrote:

Do you have any idea how much work would be necessary for the TSFlags refactor?

I think it is pretty straight forward to switch our MC code to use MCInstrDesc::TSFlags -- if we managed to rewrite our TG files to use TSFlags, which is a bigger problem. Adapting our TG code to use TSFlags requires modifying most the instruction definitions, as far as I can say.

In D88385#2476220, @Paul-C-Anagnostopoulos wrote:

If it isn't possible to encode the information in TSFlags, another possibility is to add a pointer to the MCInstrDesc class that points to some kind of auxiliary class/struct.

This actually gives me an idea to directly use the MCInstrDesc::TSFlags as a pointer to auxiliary data structures. That is:

auto* AuxTable =  reinterpret_cast<uint8_t*>(MD.TSFlags);

That means we also need to modify InstrInfoEmitter. Here is my preliminary plan:

1. In the Instruction TG class, add a new string field, ExtTSFlagsFieldName. That's say the value of this field is set to "foo"
2. If TSFlags in Instruction TG class is 0xFF...FF, InstrInfoEmitter will lookup field "foo". This "foo" field needs to be a bits, but the size need not to be 64 bits.
3. Following up step 2, InstrInfoEmitter construct auxiliary data structures according to the value of "foo" field.
4. Assign pointer to auxiliary data structure as the value of MCInstrDesc::TSFlags.

The reason there is an indirection on step 1 and 2 (i.e. use ExtTSFalgsFieldName to lookup auxiliary data's field name) is because TG's bits type can only have fixed size but we want downstream users to specify the size of their auxiliary data structure.

Biggest advantage of this approach is that we barely need to modify our TG code. We don't need to increase the size of MCInstrDesc either

I'm definitely not a TG expert, so @Paul-C-Anagnostopoulos please speak out if you have any suggestion on this approach :-)

[jrtc27]

In D88385#2476326, @myhsu wrote:

In D88385#2476220, @Paul-C-Anagnostopoulos wrote:

If it isn't possible to encode the information in TSFlags, another possibility is to add a pointer to the MCInstrDesc class that points to some kind of auxiliary class/struct.

This actually gives me an idea to directly use the MCInstrDesc::TSFlags as a pointer to auxiliary data structures. That is:

auto* AuxTable =  reinterpret_cast<uint8_t*>(MD.TSFlags);

Definitely not. Integers are not pointers, and will break on CHERI. You would need to make TSFlags a uintptr_t if you want that to work, but then you only get 32 bits on 32-bit architectures.

[myhsu]

In D88385#2476327, @jrtc27 wrote:

In D88385#2476326, @myhsu wrote:

In D88385#2476220, @Paul-C-Anagnostopoulos wrote:

If it isn't possible to encode the information in TSFlags, another possibility is to add a pointer to the MCInstrDesc class that points to some kind of auxiliary class/struct.

This actually gives me an idea to directly use the MCInstrDesc::TSFlags as a pointer to auxiliary data structures. That is:

auto* AuxTable =  reinterpret_cast<uint8_t*>(MD.TSFlags);

Definitely not. Integers are not pointers, and will break on CHERI. You would need to make TSFlags a uintptr_t if you want that to work, but then you only get 32 bits on 32-bit architectures.

Fair enough.

Then I think another way will be reusing step 1~3 in my algorithm but generate a function for MC code to access auxiliary data structures.

[Paul-C-Anagnostopoulos]

@jrtc27 Will some sort of union of a uint64_t and a pointer work? One of the Flags bits could specify which one it is.

I agree that it would be cleaner to encode the information as flags, if possible.

[jrtc27]

In D88385#2476329, @Paul-C-Anagnostopoulos wrote:

@jrtc27 Will some sort of union of a uint64_t and a pointer work? One of the Flags bits could specify which one it is.

I agree that it would be cleaner to encode the information as flags, if possible.

That'd work, yes, so long as you either have the flag bit being _zero_ mean it's a pointer or you put the flag bit sufficiently low down (preferably bit 0) as otherwise you'll take the pointer way outside the bounds of the corresponding allocation by setting a high bit; in order to fit the bounds and 64-bit address in 128 bits we compress the bounds and rely on pointers not going "too far" out of bounds (anything other than one-past-the-end is UB in C/C++, but we relax that somewhat for compatibility, roughly in proportion to the size of the allocation), but if they do then they're marked invalid and won't work later even though you mask the bit out.

[Paul-C-Anagnostopoulos]

I was thinking of using a bit in the other flags member, Flags, as opposed to TSFlags. Then there would be nothing sneaky going on in the union itself.

Making it an anonymous union would mean that all current references to TSFlags would still work, correct?

[jrtc27]

In D88385#2476359, @Paul-C-Anagnostopoulos wrote:

I was thinking of using a bit in the other flags member, Flags, as opposed to TSFlags. Then there would be nothing sneaky going on in the union itself.

Making it an anonymous union would mean that all current references to TSFlags would still work, correct?

Yes, that should all be fine.

[myhsu]

In D88385#2476359, @Paul-C-Anagnostopoulos wrote:

I was thinking of using a bit in the other flags member, Flags, as opposed to TSFlags. Then there would be nothing sneaky going on in the union itself.

Making it an anonymous union would mean that all current references to TSFlags would still work, correct?

In D88385#2476330, @jrtc27 wrote:

In D88385#2476329, @Paul-C-Anagnostopoulos wrote:

@jrtc27 Will some sort of union of a uint64_t and a pointer work? One of the Flags bits could specify which one it is.

I agree that it would be cleaner to encode the information as flags, if possible.

That'd work, yes, so long as you either have the flag bit being _zero_ mean it's a pointer or you put the flag bit sufficiently low down (preferably bit 0) as otherwise you'll take the pointer way outside the bounds of the corresponding allocation by setting a high bit; in order to fit the bounds and 64-bit address in 128 bits we compress the bounds and rely on pointers not going "too far" out of bounds (anything other than one-past-the-end is UB in C/C++, but we relax that somewhat for compatibility, roughly in proportion to the size of the allocation), but if they do then they're marked invalid and won't work later even though you mask the bit out.

Sounds like a plan, I will update this patch accordingly

Thanks for the brainstorming

[craig.topper]

In D88385#2476361, @myhsu wrote:

In D88385#2476359, @Paul-C-Anagnostopoulos wrote:

I was thinking of using a bit in the other flags member, Flags, as opposed to TSFlags. Then there would be nothing sneaky going on in the union itself.

Making it an anonymous union would mean that all current references to TSFlags would still work, correct?

In D88385#2476330, @jrtc27 wrote:

In D88385#2476329, @Paul-C-Anagnostopoulos wrote:

@jrtc27 Will some sort of union of a uint64_t and a pointer work? One of the Flags bits could specify which one it is.

I agree that it would be cleaner to encode the information as flags, if possible.

That'd work, yes, so long as you either have the flag bit being _zero_ mean it's a pointer or you put the flag bit sufficiently low down (preferably bit 0) as otherwise you'll take the pointer way outside the bounds of the corresponding allocation by setting a high bit; in order to fit the bounds and 64-bit address in 128 bits we compress the bounds and rely on pointers not going "too far" out of bounds (anything other than one-past-the-end is UB in C/C++, but we relax that somewhat for compatibility, roughly in proportion to the size of the allocation), but if they do then they're marked invalid and won't work later even though you mask the bit out.

Sounds like a plan, I will update this patch accordingly

Thanks for the brainstorming

I'm not sure I follow the plan. So we're going to change the TableGen InstrInfoEmitter to put out a pointer for this target instead of the normal TSFlags constant every other target uses?

[RKSimon]

Why would refactoring TSFlags into a int-ptr union just for M68k be better than adding the CodeBeads functionality just for M68k?

[Paul-C-Anagnostopoulos]

@RKSimon has a good question that I will leave to others to debate.

@craig.topper Actually, a pointer is a bad idea. Instead, if it is necessary to have a separate blob of data, simply use the TSFlags field as an index into a separate table of instances. It's already a uint64_t, which makes a fine index.

I presume the use of this field and the separate table would be triggered by something in a TableGen file, not just hardwired for the M86K target.

[glaubitz]

In D88385#2476491, @Paul-C-Anagnostopoulos wrote:

@RKSimon has a good question that I will leave to others to debate.

@craig.topper Actually, a pointer is a bad idea. Instead, if it is necessary to have a separate blob of data, simply use the TSFlags field as an index into a separate table of instances. It's already a uint64_t, which makes a fine index.

I presume the use of this field and the separate table would be triggered by something in a TableGen file, not just hardwired for the M86K target.

Isn't this issue something that can be resolved once the backend has been merged?

It would be great if the backend could finally be available as an experimental backend as there multiple downstream projects like the Clang Kernel project that want to test this backend.

[rengolin]

In D88385#2476435, @RKSimon wrote:

Why would refactoring TSFlags into a int-ptr union just for M68k be better than adding the CodeBeads functionality just for M68k?

I think the question is more of a "where does this belong" than "why are we doing this".

If we want to auto-generate tables at compile time, then table-gen is your friend. If this should be a bit more dynamic, then working around auto-generated tables can be cumbersome and counter productive.

The other question is "when should we do this".

We already have an implementation (CodeBeads) which I'm assuming came from an existing implementation, so (still assuming) we "know it works with m68k". The table-gen approach is so far an idea that "could" work. Working with table-gen is non-trivial.

In interest of doing one thing at a time, I'd prefer to keep the existing implementation for now and do a refactor later. As I see it, the refactor should only affect the m68k back-end.

In summary, my personal view is:

• We have something that works now and the proposed alternative is equally unique
• We should answer the "where" question before we go on implementing a new style
• If it's decided a table-gen implementation fits better, we do it as a refactory, after the merge

Putting a bit TODO comment on the current CodeBeads code will help others not try to rely on it in the interim. But if they do, then this would have proven it's more than just m68k.

[RKSimon]

FWIW I still think we're better off keeping (m68k only) CodeBeads rather than refactoring a lot of code that will affect mainstream targets. After m68k has been pushed, we can reinvestigate whether to keep CodeBeads (and whether other targets would benefit from using it), or moving m68k to a refactored TSFlag mechanism - we can make either a pre-requisite for it losing its experimental status if necessary.

[glaubitz]

In D88385#2498795, @RKSimon wrote:

FWIW I still think we're better off keeping (m68k only) CodeBeads rather than refactoring a lot of code that will affect mainstream targets. After m68k has been pushed, we can reinvestigate whether to keep CodeBeads (and whether other targets would benefit from using it), or moving m68k to a refactored TSFlag mechanism - we can make either a pre-requisite for it losing its experimental status if necessary.

I very much agree with that stance and I think this is the most straight-forward approach. I think getting the backend merged so becomes visible to a broader audience - both testers and developers - will help improve the quality and squeeze out more bugs. After all, it's supposed to be marked as experimental so it's expected to not be production-ready yet.

[jrtc27]

In D88385#2498889, @glaubitz wrote:

In D88385#2498795, @RKSimon wrote:

FWIW I still think we're better off keeping (m68k only) CodeBeads rather than refactoring a lot of code that will affect mainstream targets. After m68k has been pushed, we can reinvestigate whether to keep CodeBeads (and whether other targets would benefit from using it), or moving m68k to a refactored TSFlag mechanism - we can make either a pre-requisite for it losing its experimental status if necessary.

I very much agree with that stance and I think this is the most straight-forward approach. I think getting the backend merged so becomes visible to a broader audience - both testers and developers - will help improve the quality and squeeze out more bugs. After all, it's supposed to be marked as experimental so it's expected to not be production-ready yet.

Yeah my concern is not about m68k doing weird things, it's about the code that's added in the target-independent places to support m68k that I am most concerned with, as it affects every target regardless of whether m68k is enabled.

[RKSimon]

The impact on existing targets is pretty minimal - I suppose the logical operand mappings code is the most exposed part? The CodeBeads code is relatively self-contained.

I don't think we'll gain much by trying to iterate on this further, I'd recommend accepting the patch at this point.

[rengolin]

In D88385#2498908, @jrtc27 wrote:

Yeah my concern is not about m68k doing weird things, it's about the code that's added in the target-independent places to support m68k that I am most concerned with, as it affects every target regardless of whether m68k is enabled.

Exactly, that's why I prefer to keep the CodeBeads implementation for now (that only affects the m68k target) than refactor table-gen (that would affect all).

We should avoid mixing generic refactory with experimental code, unless the refactory is a clear benefit to all targets, not just the experimental one.

I concur with @RKSimon that we should approve this code as is and get the target in.

[jrtc27]

In D88385#2501562, @rengolin wrote:

In D88385#2498908, @jrtc27 wrote:

Yeah my concern is not about m68k doing weird things, it's about the code that's added in the target-independent places to support m68k that I am most concerned with, as it affects every target regardless of whether m68k is enabled.

Exactly, that's why I prefer to keep the CodeBeads implementation for now (that only affects the m68k target) than refactor table-gen (that would affect all).

We should avoid mixing generic refactory with experimental code, unless the refactory is a clear benefit to all targets, not just the experimental one.

I concur with @RKSimon that we should approve this code as is and get the target in.

That sounds reasonable (but +1 to @RKSimon 's comment about reviewing all this "special sauce" complexity before graduating from experimental)

[rengolin]

In D88385#2501781, @jrtc27 wrote:

That sounds reasonable (but +1 to @RKSimon 's comment about reviewing all this "special sauce" complexity before graduating from experimental)

Agreed! It should be resolved before that.

[RKSimon]

@myhsu Please can you raise a bug covering the CodeBeads vs TSFlags refactor options and make it a blocker against a second bug for making the m68k backend non-experimental?

[myhsu]

In D88385#2502280, @RKSimon wrote:

@myhsu Please can you raise a bug covering the CodeBeads vs TSFlags refactor options and make it a blocker against a second bug for making the m68k backend non-experimental?

Absolutely. I agree this problem should be solve before graduate from experimental target. I will create these bugs soon

[rengolin]

In D88385#2502299, @myhsu wrote:

Absolutely. I agree this problem should be solve before graduate from experimental target. I will create these bugs soon

You could also create a meta bug called something like "m68k in production" that all bugs that needed to be solved before we move the target to production are dependencies.

People filling bugs against the target can link to that meta bug if the issue is serious enough.

You could also create infrastructure bugs like creating buildbots, passing certain milestones in compiling programs and OSs, etc.

It'd be easy then to see that the target is good enough to be promoted to production once all those bugs (or at least all critical ones) are solved.

[myhsu]

In D88385#2503250, @rengolin wrote:

In D88385#2502299, @myhsu wrote:

Absolutely. I agree this problem should be solve before graduate from experimental target. I will create these bugs soon

You could also create a meta bug called something like "m68k in production" that all bugs that needed to be solved before we move the target to production are dependencies.

People filling bugs against the target can link to that meta bug if the issue is serious enough.

You could also create infrastructure bugs like creating buildbots, passing certain milestones in compiling programs and OSs, etc.

It'd be easy then to see that the target is good enough to be promoted to production once all those bugs (or at least all critical ones) are solved.

Alright, I've created bug 48792 for tracking it.

I've also created a meta/umbrella bug 48791 for tracking the status of graduating M68k from experimental target.

I will create more bugs regarding milestones and more OS supports and link them to the meta bug.

[RKSimon]

Thanks @myhsu!

@jrtc27 - you're still blocking this - is there anything else you think we need to address here?

[jrtc27]

In D88385#2505204, @RKSimon wrote:

Thanks @myhsu!

@jrtc27 - you're still blocking this - is there anything else you think we need to address here?

For this review, no. There are a bunch of outstanding issues on the other reviews though that need addressing; I don't know if it makes sense to land this now or wait and land them all in one go once the other reviews are fixed and approved.

[RKSimon]

In D88385#2505260, @jrtc27 wrote:

In D88385#2505204, @RKSimon wrote:

Thanks @myhsu!

@jrtc27 - you're still blocking this - is there anything else you think we need to address here?

For this review, no. There are a bunch of outstanding issues on the other reviews though that need addressing; I don't know if it makes sense to land this now or wait and land them all in one go once the other reviews are fixed and approved.

The plan is for the entire patch series to be accepted before any are pushed. Given how close we are to the next release branch (26 Jan iirc) I'd like to suggest that even if they all get accepted before then, we should delay the pushes to shortly after that.

[rengolin]

In D88385#2506204, @RKSimon wrote:

The plan is for the entire patch series to be accepted before any are pushed.

Indeed. The whole purpose of patch series is to create a dependency that we can only merge any patch once they're all approved.

Given how close we are to the next release branch (26 Jan iirc) I'd like to suggest that even if they all get accepted before then, we should delay the pushes to shortly after that.

Good point.

[myhsu]

In D88385#2506204, @RKSimon wrote:

In D88385#2505260, @jrtc27 wrote:

In D88385#2505204, @RKSimon wrote:

Thanks @myhsu!

@jrtc27 - you're still blocking this - is there anything else you think we need to address here?

For this review, no. There are a bunch of outstanding issues on the other reviews though that need addressing; I don't know if it makes sense to land this now or wait and land them all in one go once the other reviews are fixed and approved.

The plan is for the entire patch series to be accepted before any are pushed.

Given how close we are to the next release branch (26 Jan iirc) I'd like to suggest that even if they all get accepted before then, we should delay the pushes to shortly after that.

Good point! I'll wait until the release is over

[RKSimon]

@myhsu I think you will also need a Patch 0/8 - adding you as the m68k code owner :)

[myhsu]

In D88385#2518064, @RKSimon wrote:

@myhsu I think you will also need a Patch 0/8 - adding you as the m68k code owner :)

Yes you're right :-) Will do.

[jrtc27]

Some minor remaining issues, but this is fine to commit once those are fixed IMO.

llvm/utils/TableGen/CodeBeadsGen.cpp

18

Still outstanding

55

95–96

Triple-slash is for doxygen

llvm/utils/TableGen/InstrInfoEmitter.cpp

512

531

610

[myhsu]

• Addressed feedbacks

llvm/include/llvm/Target/Target.td

class Instruction : InstructionEncoding {
 
   ///@}
 
   /// UseNamedOperandTable - If set, the operand indices of this instruction
   /// can be queried via the getNamedOperandIdx() function which is generated
   /// by TableGen.
   bit UseNamedOperandTable = false;
 
+  /// Should generate helper functions that help you to map a logical operand's
+  /// index to the underlying MIOperand's index.
+  /// In most architectures logical operand indicies are equal to
+  /// MIOperand indicies, but for some CISC architectures, a logical operand
+  /// might be consist of multiple MIOperand (e.g. a logical operand that
+  /// uses complex address mode).
+  bit UseLogicalOperandMappings = false;
+
   /// Should FastISel ignore this instruction. For certain ISAs, they have
   /// instructions which map to the same ISD Opcode, value type operands and
   /// instruction selection predicates. FastISel cannot handle such cases, but
   /// SelectionDAG can.
   bit FastISelShouldIgnore = false;
 }
 
 /// Defines an additional encoding that disassembles to the given instruction

llvm/utils/TableGen/CMakeLists.txt

 add_subdirectory(GlobalISel)
 
 set(LLVM_LINK_COMPONENTS Support)
 
 add_tablegen(llvm-tblgen LLVM
   AsmMatcherEmitter.cpp
   AsmWriterEmitter.cpp
   AsmWriterInst.cpp
   Attributes.cpp
   CallingConvEmitter.cpp
+  CodeBeadsGen.cpp
   CodeEmitterGen.cpp
   CodeGenDAGPatterns.cpp
   CodeGenHwModes.cpp
   CodeGenInstruction.cpp
   CodeGenMapTable.cpp
   CodeGenRegisters.cpp
   CodeGenSchedule.cpp
   CodeGenTarget.cpp
add_tablegen(llvm-tblgen LLVM
   Types.cpp
   X86DisassemblerTables.cpp
   X86EVEX2VEXTablesEmitter.cpp
   X86FoldTablesEmitter.cpp
   X86ModRMFilters.cpp
   X86RecognizableInstr.cpp
   WebAssemblyDisassemblerEmitter.cpp
   CTagsEmitter.cpp
   )
 target_link_libraries(llvm-tblgen PRIVATE LLVMTableGenGlobalISel)
 set_target_properties(llvm-tblgen PROPERTIES FOLDER "Tablegenning")

llvm/utils/TableGen/CodeBeadsGen.cpp

+//===---------- CodeBeadsGen.cpp - Code Beads Generator -------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+// CodeBeads are data fields carrying auxiliary information for instructions.
+//
+// Under the hood it's simply implemented by a `bits` field (with arbitrary
+// length) in each TG instruction description, where this TG backend will
+// generate a helper function to access it.
+//
+// This is especially useful for expressing variable length encoding
+// instructions and complex addressing modes. Since in those cases each
+// instruction is usually associated with large amount of information like
+// addressing mode details used on a specific operand. Instead of retreating to
+// ad-hoc methods to figure out these information when encoding an instruction,
+// CodeBeads provide a clean table for the instruction encoder to lookup.
+//===----------------------------------------------------------------------===//
+
+#include "CodeGenTarget.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/TableGen/Error.h"
+#include "llvm/TableGen/Record.h"
+#include "llvm/TableGen/TableGenBackend.h"
+#include <map>
+#include <string>
+#include <vector>
+using namespace llvm;
+
+namespace {
+
+class CodeBeadsGen {
+  RecordKeeper &Records;
+
+public:
+  CodeBeadsGen(RecordKeeper &R) : Records(R) {}
+  void run(raw_ostream &OS);
+};
+
+void CodeBeadsGen::run(raw_ostream &OS) {
+  CodeGenTarget Target(Records);
+  std::vector<Record *> Insts = Records.getAllDerivedDefinitions("Instruction");
+
+  // For little-endian instruction bit encodings, reverse the bit order
+  Target.reverseBitsForLittleEndianEncoding();
+
+  ArrayRef<const CodeGenInstruction *> NumberedInstructions =
+      Target.getInstructionsByEnumValue();
+
+  // Emit function declaration
+  OS << "const uint8_t *llvm::" << Target.getInstNamespace();
+  OS << "::getMCInstrBeads(unsigned Opcode) {\n";
+
+  // First, get the maximum bit length among all beads. And do some
+  // simple validation
+  unsigned MaxBitLength = 0;
+
+  for (const CodeGenInstruction *CGI : NumberedInstructions) {
+    Record *R = CGI->TheDef;
+    if (!R->getValue("Beads"))
+      continue;
+
+    BitsInit *BI = R->getValueAsBitsInit("Beads");
+    if (!BI->isComplete()) {
+      PrintFatalError(R->getLoc(), "Record `" + R->getName() +
+                                       "', bit field 'Beads' is not complete");
+    }
+
+    MaxBitLength = std::max(MaxBitLength, BI->getNumBits());
+  }
+
+  // Number of bytes
+  unsigned Parts = MaxBitLength / 8;
+
+  // Emit instruction base values
+  OS << "  static const uint8_t InstBits[][" << Parts << "] = {\n";
+  for (const CodeGenInstruction *CGI : NumberedInstructions) {
+    Record *R = CGI->TheDef;
+
+    if (R->getValueAsString("Namespace") == "TargetOpcode" ||
+        !R->getValue("Beads")) {
+      OS << "\t{ 0x0 },\t// ";
+      if (R->getValueAsBit("isPseudo"))
+        OS << "(Pseudo) ";
+      OS << R->getName() << "\n";
+      continue;
+    }
+
+    BitsInit *BI = R->getValueAsBitsInit("Beads");
+
+    // Convert to byte array:
+    // [dcba] -> [a][b][c][d]
+    OS << "\t{";
+    for (unsigned p = 0; p < Parts; ++p) {
+      unsigned Right = 8 * p;
+      unsigned Left = Right + 8;
+
+      uint8_t Value = 0;
+      for (unsigned i = Right; i != Left; ++i) {
+        unsigned Shift = i % 8;
+        if (auto *B = dyn_cast<BitInit>(BI->getBit(i))) {
+          Value |= (static_cast<uint8_t>(B->getValue()) << Shift);
+        } else {
+          PrintFatalError(R->getLoc(), "Record `" + R->getName() +
+                                           "', bit 'Beads[" + Twine(i) +
+                                           "]' is not defined");
+        }
+      }
+
+      if (p)
+        OS << ',';
+      OS << " 0x";
+      OS.write_hex(Value);
+      OS << "";
+    }
+    OS << " }," << '\t' << "// " << R->getName() << "\n";
+  }
+  OS << "\t{ 0x0 }\n  };\n";
+
+  // Emit initial function code
+  OS << "  return InstBits[Opcode];\n"
+     << "}\n\n";
+}
+
+} // End anonymous namespace
+
+namespace llvm {
+
+void EmitCodeBeads(RecordKeeper &RK, raw_ostream &OS) {
+  emitSourceFileHeader("Machine Code Beads", OS);
+  CodeBeadsGen(RK).run(OS);
+}
+
+} // namespace llvm

llvm/utils/TableGen/InstrInfoEmitter.cpp

 #include "CodeGenDAGPatterns.h"
 #include "CodeGenInstruction.h"
 #include "CodeGenSchedule.h"
 #include "CodeGenTarget.h"
 #include "PredicateExpander.h"
 #include "SequenceToOffsetTable.h"
 #include "TableGenBackends.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TableGen/Error.h"
 #include "llvm/TableGen/Record.h"
 #include "llvm/TableGen/TableGenBackend.h"
 #include <cassert>
 #include <cstdint>
+#include <iterator>
 #include <map>
 #include <string>
 #include <utility>
 #include <vector>
 
 using namespace llvm;
 
 namespace {
private:
   void initOperandMapData(
             ArrayRef<const CodeGenInstruction *> NumberedInstructions,
             StringRef Namespace,
             std::map<std::string, unsigned> &Operands,
             OpNameMapTy &OperandMap);
   void emitOperandNameMappings(raw_ostream &OS, const CodeGenTarget &Target,
             ArrayRef<const CodeGenInstruction*> NumberedInstructions);
 
+  void emitLogicalOperandSizeMappings(
+      raw_ostream &OS, StringRef Namespace,
+      ArrayRef<const CodeGenInstruction *> NumberedInstructions);
+  void emitLogicalOperandTypeMappings(
+      raw_ostream &OS, StringRef Namespace,
+      ArrayRef<const CodeGenInstruction *> NumberedInstructions);
+
   // Operand information.
   void EmitOperandInfo(raw_ostream &OS, OperandInfoMapTy &OperandInfoIDs);
   std::vector<std::string> GetOperandInfo(const CodeGenInstruction &Inst);
 };
 
 } // end anonymous namespace
 
 static void PrintDefList(const std::vector<Record*> &Uses,
if (!NumberedInstructions.empty()) {
     OS << "  llvm_unreachable(\"No instructions defined\");\n";
   }
   OS << "}\n";
   OS << "} // end namespace " << Namespace << "\n";
   OS << "} // end namespace llvm\n";
   OS << "#endif // GET_INSTRINFO_OPERAND_TYPE\n\n";
 }
 
+void InstrInfoEmitter::emitLogicalOperandSizeMappings(
+    raw_ostream &OS, StringRef Namespace,
+    ArrayRef<const CodeGenInstruction *> NumberedInstructions) {
+  std::map<std::vector<unsigned>, unsigned> LogicalOpSizeMap;
+
+  std::map<unsigned, std::vector<std::string>> InstMap;
+
+  size_t LogicalOpListSize = 0U;
+  std::vector<unsigned> LogicalOpList;
+  for (const auto *Inst : NumberedInstructions) {
+    if (!Inst->TheDef->getValueAsBit("UseLogicalOperandMappings"))
+      continue;
+
+    LogicalOpList.clear();
+    llvm::transform(Inst->Operands, std::back_inserter(LogicalOpList),
+                    [](const CGIOperandList::OperandInfo &Op) -> unsigned {
+                      auto *MIOI = Op.MIOperandInfo;
+                      if (!MIOI || MIOI->getNumArgs() == 0)
+                        return 1;
+                      return MIOI->getNumArgs();
+                    });
+    LogicalOpListSize = std::max(LogicalOpList.size(), LogicalOpListSize);
+
+    auto I =
+        LogicalOpSizeMap.insert({LogicalOpList, LogicalOpSizeMap.size()}).first;
+    InstMap[I->second].push_back(
+        (Namespace + "::" + Inst->TheDef->getName()).str());
+  }
+
+  OS << "#ifdef GET_INSTRINFO_LOGICAL_OPERAND_SIZE_MAP\n";
+  OS << "#undef GET_INSTRINFO_LOGICAL_OPERAND_SIZE_MAP\n";
+  OS << "namespace llvm {\n";
+  OS << "namespace " << Namespace << " {\n";
+  OS << "LLVM_READONLY static unsigned\n";
+  OS << "getLogicalOperandSize(uint16_t Opcode, uint16_t LogicalOpIdx) {\n";
+  if (!InstMap.empty()) {
+    std::vector<const std::vector<unsigned> *> LogicalOpSizeList(
+        LogicalOpSizeMap.size());
+    for (auto &P : LogicalOpSizeMap) {
+      LogicalOpSizeList[P.second] = &P.first;
+    }
+    OS << "  static const unsigned SizeMap[][" << LogicalOpListSize
+       << "] = {\n";
+    for (int r = 0, rs = LogicalOpSizeList.size(); r < rs; ++r) {
+      const auto &Row = *LogicalOpSizeList[r];
+      OS << "   {";
+      int i;
+      for (i = 0; i < static_cast<int>(Row.size()); ++i) {
+        OS << Row[i] << ", ";
+      }
+      for (; i < static_cast<int>(LogicalOpListSize); ++i) {
+        OS << "0, ";
+      }
+      OS << "}, ";
+      OS << "\n";
+    }
+    OS << "  };\n";
+
+    OS << "  switch (Opcode) {\n";
+    OS << "  default: return LogicalOpIdx;\n";
+    for (auto &P : InstMap) {
+      auto OpMapIdx = P.first;
+      const auto &Insts = P.second;
+      for (const auto &Inst : Insts) {
+        OS << "  case " << Inst << ":\n";
+      }
+      OS << "    return SizeMap[" << OpMapIdx << "][LogicalOpIdx];\n";
+    }
+    OS << "  }\n";
+  } else {
+    OS << "  return LogicalOpIdx;\n";
+  }
+  OS << "}\n";
+
+  OS << "LLVM_READONLY static inline unsigned\n";
+  OS << "getLogicalOperandIdx(uint16_t Opcode, uint16_t LogicalOpIdx) {\n";
+  OS << "  auto S = 0U;\n";
+  OS << "  for (auto i = 0U; i < LogicalOpIdx; ++i)\n";
+  OS << "    S += getLogicalOperandSize(Opcode, i);\n";
+  OS << "  return S;\n";
+  OS << "}\n";
+
+  OS << "} // end namespace " << Namespace << "\n";
+  OS << "} // end namespace llvm\n";
+  OS << "#endif // GET_INSTRINFO_LOGICAL_OPERAND_SIZE_MAP\n\n";
+}
+
+void InstrInfoEmitter::emitLogicalOperandTypeMappings(
+    raw_ostream &OS, StringRef Namespace,
+    ArrayRef<const CodeGenInstruction *> NumberedInstructions) {
+  std::map<std::vector<std::string>, unsigned> LogicalOpTypeMap;
+
+  std::map<unsigned, std::vector<std::string>> InstMap;
+
+  size_t OpTypeListSize = 0U;
+  std::vector<std::string> LogicalOpTypeList;
+  for (const auto *Inst : NumberedInstructions) {
+    if (!Inst->TheDef->getValueAsBit("UseLogicalOperandMappings"))
+      continue;
+
+    LogicalOpTypeList.clear();
+    for (const auto &Op : Inst->Operands) {
+      auto *OpR = Op.Rec;
+      if ((OpR->isSubClassOf("Operand") ||
+           OpR->isSubClassOf("RegisterOperand") ||
+           OpR->isSubClassOf("RegisterClass")) &&
+          !OpR->isAnonymous()) {
+        LogicalOpTypeList.push_back(
+            (Namespace + "::OpTypes::" + Op.Rec->getName()).str());
+      } else {
+        LogicalOpTypeList.push_back("-1");
+      }
+    }
+    OpTypeListSize = std::max(LogicalOpTypeList.size(), OpTypeListSize);
+
+    auto I =
+        LogicalOpTypeMap.insert({LogicalOpTypeList, LogicalOpTypeMap.size()})
+            .first;
+    InstMap[I->second].push_back(
+        (Namespace + "::" + Inst->TheDef->getName()).str());
+  }
+
+  OS << "#ifdef GET_INSTRINFO_LOGICAL_OPERAND_TYPE_MAP\n";
+  OS << "#undef GET_INSTRINFO_LOGICAL_OPERAND_TYPE_MAP\n";
+  OS << "namespace llvm {\n";
+  OS << "namespace " << Namespace << " {\n";
+  OS << "LLVM_READONLY static int\n";
+  OS << "getLogicalOperandType(uint16_t Opcode, uint16_t LogicalOpIdx) {\n";
+  if (!InstMap.empty()) {
+    std::vector<const std::vector<std::string> *> LogicalOpTypeList(
+        LogicalOpTypeMap.size());
+    for (auto &P : LogicalOpTypeMap) {
+      LogicalOpTypeList[P.second] = &P.first;
+    }
+    OS << "  static const int TypeMap[][" << OpTypeListSize << "] = {\n";
+    for (int r = 0, rs = LogicalOpTypeList.size(); r < rs; ++r) {
+      const auto &Row = *LogicalOpTypeList[r];
+      OS << "   {";
+      int i, s = Row.size();
+      for (i = 0; i < s; ++i) {
+        if (i > 0)
+          OS << ", ";
+        OS << Row[i];
+      }
+      for (; i < static_cast<int>(OpTypeListSize); ++i) {
+        if (i > 0)
+          OS << ", ";
+        OS << "-1";
+      }
+      OS << "}";
+      if (r != rs - 1)
+        OS << ",";
+      OS << "\n";
+    }
+    OS << "  };\n";
+
+    OS << "  switch (Opcode) {\n";
+    OS << "  default: return -1;\n";
+    for (auto &P : InstMap) {
+      auto OpMapIdx = P.first;
+      const auto &Insts = P.second;
+      for (const auto &Inst : Insts) {
+        OS << "  case " << Inst << ":\n";
+      }
+      OS << "    return TypeMap[" << OpMapIdx << "][LogicalOpIdx];\n";
+    }
+    OS << "  }\n";
+  } else {
+    OS << "  return -1;\n";
+  }
+  OS << "}\n";
+  OS << "} // end namespace " << Namespace << "\n";
+  OS << "} // end namespace llvm\n";
+  OS << "#endif // GET_INSTRINFO_LOGICAL_OPERAND_TYPE_MAP\n\n";
+}
+
 void InstrInfoEmitter::emitMCIIHelperMethods(raw_ostream &OS,
                                              StringRef TargetName) {
   RecVec TIIPredicates = Records.getAllDerivedDefinitions("TIIPredicate");
   if (TIIPredicates.empty())
     return;
 
   OS << "#ifdef GET_INSTRINFO_MC_HELPER_DECLS\n";
   OS << "#undef GET_INSTRINFO_MC_HELPER_DECLS\n\n";
void InstrInfoEmitter::run(raw_ostream &OS) {
   OS << "#endif // GET_INSTRINFO_CTOR_DTOR\n\n";
 
   Records.startTimer("Emit operand name mappings");
   emitOperandNameMappings(OS, Target, NumberedInstructions);
 
   Records.startTimer("Emit operand type mappings");
   emitOperandTypeMappings(OS, Target, NumberedInstructions);
 
+  Records.startTimer("Emit logical operand size mappings");
+  emitLogicalOperandSizeMappings(OS, TargetName, NumberedInstructions);
+
+  Records.startTimer("Emit logical operand type mappings");
+  emitLogicalOperandTypeMappings(OS, TargetName, NumberedInstructions);
+
   Records.startTimer("Emit helper methods");
   emitMCIIHelperMethods(OS, TargetName);
 }
 
 void InstrInfoEmitter::emitRecord(const CodeGenInstruction &Inst, unsigned Num,
                                   Record *InstrInfo,
                          std::map<std::vector<Record*>, unsigned> &EmittedLists,
                                   const OperandInfoMapTy &OpInfo,

llvm/utils/TableGen/TableGen.cpp

 using namespace llvm;
 
 enum ActionType {
   PrintRecords,
   PrintDetailedRecords,
   NullBackend,
   DumpJSON,
   GenEmitter,
+  GenCodeBeads,
   GenRegisterInfo,
   GenInstrInfo,
   GenInstrDocs,
   GenAsmWriter,
   GenAsmMatcher,
   GenDisassembler,
   GenPseudoLowering,
   GenCompressInst,
cl::values(
                    "Print all records to stdout (default)"),
         clEnumValN(PrintDetailedRecords, "print-detailed-records",
                    "Print full details of all records to stdout"),
         clEnumValN(NullBackend, "null-backend",
                    "Do nothing after parsing (useful for timing)"),
         clEnumValN(DumpJSON, "dump-json",
                    "Dump all records as machine-readable JSON"),
         clEnumValN(GenEmitter, "gen-emitter", "Generate machine code emitter"),
+        clEnumValN(GenCodeBeads, "gen-code-beads",
+                   "Generate machine code beads"),
         clEnumValN(GenRegisterInfo, "gen-register-info",
                    "Generate registers and register classes info"),
         clEnumValN(GenInstrInfo, "gen-instr-info",
                    "Generate instruction descriptions"),
         clEnumValN(GenInstrDocs, "gen-instr-docs",
                    "Generate instruction documentation"),
         clEnumValN(GenCallingConv, "gen-callingconv",
                    "Generate calling convention descriptions"),
bool LLVMTableGenMain(raw_ostream &OS, RecordKeeper &Records) {
   case NullBackend:             // No backend at all.
     break;
   case DumpJSON:
     EmitJSON(Records, OS);
     break;
   case GenEmitter:
     EmitCodeEmitter(Records, OS);
     break;
+  case GenCodeBeads:
+    EmitCodeBeads(Records, OS);
+    break;
   case GenRegisterInfo:
     EmitRegisterInfo(Records, OS);
     break;
   case GenInstrInfo:
     EmitInstrInfo(Records, OS);
     break;
   case GenInstrDocs:
     EmitInstrDocs(Records, OS);

llvm/utils/TableGen/TableGenBackends.h

 class RecordKeeper;
 
 void EmitIntrinsicEnums(RecordKeeper &RK, raw_ostream &OS);
 void EmitIntrinsicImpl(RecordKeeper &RK, raw_ostream &OS);
 void EmitAsmMatcher(RecordKeeper &RK, raw_ostream &OS);
 void EmitAsmWriter(RecordKeeper &RK, raw_ostream &OS);
 void EmitCallingConv(RecordKeeper &RK, raw_ostream &OS);
 void EmitCodeEmitter(RecordKeeper &RK, raw_ostream &OS);
+void EmitCodeBeads(RecordKeeper &RK, raw_ostream &OS);
 void EmitDAGISel(RecordKeeper &RK, raw_ostream &OS);
 void EmitDFAPacketizer(RecordKeeper &RK, raw_ostream &OS);
 void EmitDisassembler(RecordKeeper &RK, raw_ostream &OS);
 void EmitFastISel(RecordKeeper &RK, raw_ostream &OS);
 void EmitInstrInfo(RecordKeeper &RK, raw_ostream &OS);
 void EmitInstrDocs(RecordKeeper &RK, raw_ostream &OS);
 void EmitPseudoLowering(RecordKeeper &RK, raw_ostream &OS);
 void EmitCompressInst(RecordKeeper &RK, raw_ostream &OS);