This is an archive of the discontinued LLVM Phabricator instance.

[TableGen] AsmMatcherEmitter.cpp: replace a sequence of "if" to "switch" in emitValidateOperandClass. NFC.
ClosedPublic

Authored by vpykhtin on Mar 23 2016, 7:30 AM.

Details

Summary

Hi,

this NFC change replaces a sequence of generated

if (Kind == MCK_XXX) {

if (Operand.isXXX())
   return MCTargetAsmParser::Match_Success;

}

to

switch (Kind) {
default: break;
...
case MCK_xxx: ...

if (Operand.isXXX())
   return MCTargetAsmParser::Match_Success;

break;
...
}

Potentially can generate faster Kind check and significantly eases step debugging.

Example of output (from AMDGPU backend):

switch(Kind) {
default: break;
// 'Clamp' class
case MCK_Clamp:
  if (Operand.isClamp())
    return MCTargetAsmParser::Match_Success;
  break;
// 'DMask' class
case MCK_DMask:
  if (Operand.isDMask())
    return MCTargetAsmParser::Match_Success;
  break;

...

} // end switch(Kind)

Diff Detail

Repository
rL LLVM

Event Timeline

vpykhtin updated this revision to Diff 51415.Mar 23 2016, 7:30 AM
vpykhtin retitled this revision from to [TableGen] AsmMatcherEmitter.cpp: replace a sequence of "if" to "switch" in emitValidateOperandClass. NFC..
vpykhtin updated this object.
vpykhtin added a subscriber: llvm-commits.
ab added a subscriber: ab.Mar 28 2016, 11:32 AM

Nicer indeed! One question and one nit.

utils/TableGen/AsmMatcherEmitter.cpp
2156 ↗(On Diff #51415)

space before '('

2157 ↗(On Diff #51415)

Do we ever hit the default? Perhaps you could put an llvm_unreachable().

vpykhtin added a reviewer: ab.Mar 28 2016, 1:11 PM
In D18394#384788, @ab wrote:

Nicer indeed! One question and one nit.

Thanks for looking into this, default is a very common case - it handles registers below the switch. Will fix space on submit.

ab accepted this revision.Apr 4 2016, 5:46 PM
ab edited edge metadata.

SGTM

This revision is now accepted and ready to land.Apr 4 2016, 5:46 PM
This revision was automatically updated to reflect the committed changes.