This is an archive of the discontinued LLVM Phabricator instance.

[SystemZ] Adding extra extended mnemonics for SystemZ target
ClosedPublic

Authored by anirudhp on Nov 26 2020, 8:42 AM.

Details

Summary
  • This patch consists of the addition of some common additional extended mnemonics to the SystemZ target
  • These are jnop, jct, jctg, jas, jasl, jxh, jxhg, jxle, jxleg, bru, brul, br*, br*l.
  • These mnemonics and the instructions they map to are defined here, Chapter 4 - Branching with extended mnemonic codes
  • Except for jnop (which is a variant of brc 0, label), every other mnemonic is marked as a MnemonicAlias since there is already a "defined" instruction with the same encoding and/or condition mask values
  • brc 0, label doesn't have a defined extended mnemonic, thus jnop is defined using as an InstAlias.
  • Furthermore, the applyMnemonicAliases function is called in the overridden parseInstruction function in SystemZAsmParser.cpp to ensure any mnemonic aliases are applied before any further processing on the instruction is done.

Diff Detail

Event Timeline

anirudhp created this revision.Nov 26 2020, 8:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 26 2020, 8:42 AM
anirudhp requested review of this revision.Nov 26 2020, 8:42 AM

This looks good to me. The only thing I'm wondering is whether we shouldn't complete the NOP set by adding a 6-byte "jgnop". (I guess this is called jlnop with HLASM, but in order to keep with the jg* naming scheme in GAS we probably ought to use jgnop here.)

anirudhp updated this revision to Diff 308058.Nov 27 2020, 7:57 AM
  • Added the jgnop extended mnemonic. This is an alias to brcl 0, label. brcl 0, label also doesn't have a defined extended mnemonic, thus jgnop is also defined as using an InstAlias

This looks good to me. The only thing I'm wondering is whether we shouldn't complete the NOP set by adding a 6-byte "jgnop". (I guess this is called jlnop with HLASM, but in order to keep with the jg* naming scheme in GAS we probably ought to use jgnop here.)

This is a good idea. I have updated the patch to reflect the same. Thanks for the hint!

uweigand accepted this revision.Nov 27 2020, 9:03 AM

LGTM, thanks!

This revision is now accepted and ready to land.Nov 27 2020, 9:03 AM

@uweigand If you are happy with this patch and have no other comments for me to address, would you please be able to commit this? (Since I don't have commit access)

This revision was automatically updated to reflect the committed changes.