[llvm-commits] [llvm] r113163 - in /llvm/trunk: lib/Target/ARM/AsmParser/ARMAsmParser.cpp lib/Target/X86/AsmParser/X86AsmLexer.cpp lib/Target/X86/AsmParser/X86AsmParser.cpp utils/TableGen/AsmMatcherEmitter.cpp

Daniel Dunbar daniel at zuster.org
Fri Feb 4 10:45:44 PST 2011


Hi Chris,

On Mon, Sep 6, 2010 at 12:11 PM, Chris Lattner <sabre at nondot.org> wrote:
> Author: lattner
> Date: Mon Sep  6 14:11:01 2010
> New Revision: 113163
>
> URL: http://llvm.org/viewvc/llvm-project?rev=113163&view=rev
> Log:
> have AsmMatcherEmitter.cpp produce the hunk of code that gets included
> into the middle of the class, and rework how the different sections of
> the generated file are conditionally included for simplicity.

I personally find this to be a bit gross, it makes things more
obscure. In particular, in the current incarnation certain types
(MatchResultTy) which are static and could be defined in a common .h
file are in an autogenerated file.

I think there are a better approaches than using multiple inclusion:

(1) Make these things proper pure virtual functions in the
TargetAsmParser base class, and have the clients override them.
Requires static casting of the ComputeAvailableFeatures argument, but
this isn't a big deal.

Pro: static type checking with good diagnostics.

(2) Even if you don't want to do (1), it would still be cleaner to
just generate multiple files in the matcher emitter, than to generate
one file which is included multiple times.

This isn't a big deal, of course...

 - Daniel

>
> Modified:
>    llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
>    llvm/trunk/lib/Target/X86/AsmParser/X86AsmLexer.cpp
>    llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp
>    llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp
>
> Modified: llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp?rev=113163&r1=113162&r2=113163&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp Mon Sep  6 14:11:01 2010
> @@ -95,11 +95,8 @@
>   /// @name Auto-generated Match Functions
>   /// {
>
> -  unsigned ComputeAvailableFeatures(const ARMSubtarget *Subtarget) const;
> -
> -  bool MatchInstructionImpl(const SmallVectorImpl<MCParsedAsmOperand*>
> -                              &Operands,
> -                            MCInst &Inst);
> +#define GET_ASSEMBLER_HEADER
> +#include "ARMGenAsmMatcher.inc"
>
>   /// }
>
> @@ -869,4 +866,6 @@
>   LLVMInitializeARMAsmLexer();
>  }
>
> +#define GET_REGISTER_MATCHER
> +#define GET_MATCHER_IMPLEMENTATION
>  #include "ARMGenAsmMatcher.inc"
>
> Modified: llvm/trunk/lib/Target/X86/AsmParser/X86AsmLexer.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/AsmParser/X86AsmLexer.cpp?rev=113163&r1=113162&r2=113163&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/AsmParser/X86AsmLexer.cpp (original)
> +++ llvm/trunk/lib/Target/X86/AsmParser/X86AsmLexer.cpp Mon Sep  6 14:11:01 2010
> @@ -65,9 +65,10 @@
>   }
>  };
>
> -}
> +} // end anonymous namespace
>
> -static unsigned MatchRegisterName(StringRef Name);
> +#define GET_REGISTER_MATCHER
> +#include "X86GenAsmMatcher.inc"
>
>  AsmToken X86AsmLexer::LexTokenATT() {
>   AsmToken lexedToken = lexDefinite();
> @@ -162,7 +163,3 @@
>   RegisterAsmLexer<X86AsmLexer> X(TheX86_32Target);
>   RegisterAsmLexer<X86AsmLexer> Y(TheX86_64Target);
>  }
> -
> -#define REGISTERS_ONLY
> -#include "X86GenAsmMatcher.inc"
> -#undef REGISTERS_ONLY
>
> Modified: llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp?rev=113163&r1=113162&r2=113163&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp (original)
> +++ llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp Mon Sep  6 14:11:01 2010
> @@ -56,12 +56,10 @@
>
>   /// @name Auto-generated Matcher Functions
>   /// {
> -
> -  unsigned ComputeAvailableFeatures(const X86Subtarget *Subtarget) const;
> -
> -  bool MatchInstructionImpl(
> -    const SmallVectorImpl<MCParsedAsmOperand*> &Operands, MCInst &Inst);
> -
> +
> +#define GET_ASSEMBLER_HEADER
> +#include "X86GenAsmMatcher.inc"
> +
>   /// }
>
>  public:
> @@ -882,9 +880,6 @@
>                                   MCInst &Inst) {
>   assert(!Operands.empty() && "Unexpect empty operand list!");
>
> -  X86Operand *Op = static_cast<X86Operand*>(Operands[0]);
> -  assert(Op->isToken() && "Leading operand should always be a mnemonic!");
> -
>   // First, try a direct match.
>   if (!MatchInstructionImpl(Operands, Inst))
>     return false;
> @@ -894,6 +889,9 @@
>   // type. However, that requires substantially more matcher support than the
>   // following hack.
>
> +  X86Operand *Op = static_cast<X86Operand*>(Operands[0]);
> +  assert(Op->isToken() && "Leading operand should always be a mnemonic!");
> +
>   // Change the operand to point to a temporary token.
>   StringRef Base = Op->getToken();
>   SmallString<16> Tmp;
> @@ -966,4 +964,6 @@
>   LLVMInitializeX86AsmLexer();
>  }
>
> +#define GET_REGISTER_MATCHER
> +#define GET_MATCHER_IMPLEMENTATION
>  #include "X86GenAsmMatcher.inc"
>
> Modified: llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp?rev=113163&r1=113162&r2=113163&view=diff
> ==============================================================================
> --- llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp (original)
> +++ llvm/trunk/utils/TableGen/AsmMatcherEmitter.cpp Mon Sep  6 14:11:01 2010
> @@ -1541,13 +1541,32 @@
>
>   EmitSourceFileHeader("Assembly Matcher Source Fragment", OS);
>
> +  // Information for the class declaration.
> +  OS << "\n#ifdef GET_ASSEMBLER_HEADER\n";
> +  OS << "#undef GET_ASSEMBLER_HEADER\n";
> +  OS << "  unsigned ComputeAvailableFeatures(const " <<
> +           Target.getName() << "Subtarget *Subtarget) const;\n";
> +  OS << "bool MatchInstructionImpl(const SmallVectorImpl<MCParsedAsmOperand*>"
> +     << " &Operands, MCInst &Inst);\n\n";
> +  OS << "#endif // GET_ASSEMBLER_HEADER_INFO\n\n";
> +
> +
> +
> +
> +  OS << "\n#ifdef GET_REGISTER_MATCHER\n";
> +  OS << "#undef GET_REGISTER_MATCHER\n\n";
> +
>   // Emit the subtarget feature enumeration.
>   EmitSubtargetFeatureFlagEnumeration(Target, Info, OS);
>
>   // Emit the function to match a register name to number.
>   EmitMatchRegisterName(Target, AsmParser, OS);
> +
> +  OS << "#endif // GET_REGISTER_MATCHER\n\n";
>
> -  OS << "#ifndef REGISTERS_ONLY\n\n";
> +
> +  OS << "\n#ifdef GET_MATCHER_IMPLEMENTATION\n";
> +  OS << "#undef GET_MATCHER_IMPLEMENTATION\n\n";
>
>   // Generate the unified function to convert operands into an MCInst.
>   EmitConvertToMCInst(Target, Info.Instructions, OS);
> @@ -1658,16 +1677,17 @@
>      << "; it != ie; ++it) {\n";
>
>   // Emit check that the required features are available.
> -    OS << "    if ((AvailableFeatures & it->RequiredFeatures) "
> -       << "!= it->RequiredFeatures)\n";
> -    OS << "      continue;\n";
> -
> +  OS << "    if ((AvailableFeatures & it->RequiredFeatures) "
> +     << "!= it->RequiredFeatures)\n";
> +  OS << "      continue;\n";
> +
>   // Emit check that the subclasses match.
>   for (unsigned i = 0; i != MaxNumOperands; ++i) {
>     OS << "    if (!IsSubclass(Classes["
>        << i << "], it->Classes[" << i << "]))\n";
>     OS << "      continue;\n";
>   }
> +
>   OS << "\n";
>   OS << "    ConvertToMCInst(it->ConvertFn, Inst, it->Opcode, Operands);\n";
>
> @@ -1683,5 +1703,5 @@
>   OS << "  return true;\n";
>   OS << "}\n\n";
>
> -  OS << "#endif // REGISTERS_ONLY\n";
> +  OS << "#endif // GET_MATCHER_IMPLEMENTATION\n\n";
>  }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>




More information about the llvm-commits mailing list