[PATCH 1/2] TableGen: Generate a function for getting operand indices based on their defined names

[Sean Silva]

On Mon, Jun 24, 2013 at 12:22 PM, Tom Stellard <tom at stellard.net> wrote:

> Ping.
>
>
Oops, sorry.

+/// \p Operands [out] A map used to generate the OpName enum with operand
+///     names as its keys and operand enum values as its values.
+///  \p OperandMap [out] A map for representing the operand name mappings
for
+///     each instructions.  This is used to generate the OperandMap table
as
+///     well as the getNamedOperandIdx() function.

These are aligned differently. Also, the prevailing style in the codebase
seems to be `\param`.

+  for (unsigned i = 0, e = NumberedInstructions.size(), NumOperands = 0;
+                                                                  i != e;
++i) {

Could you pull NumOperands onto its own line before the `for` like you did
for `TableIndex` in another part of the code? I think that's a lot more
readable since it doesn't hide an unrelated variable inside the `for`
header.

+        const std::vector<const CodeGenInstruction *> NumberedInstructions,
+        std::string Namespace,

+  std::string Namespace = Target.getInstNamespace();

Do you need to be making copies here?

+    for (unsigned i = 0, e = Operands.size(); i != e; ++i)

I think this will provoke a -Wshadow warning. Best to rename the nested
loop variables like you did elsewhere (also, it would be nice if you were
consistent in the naming of the nested loop counters (`j`, `je` vs. `ii`,
`ie`)).

Other than that, it looks good to me. Jakob, any other thoughts?

-- Sean Silva
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130624/78be8a37/attachment.html>