[PATCH] D48527: [X86] Sort the static memory folding tables by reg opcode. Remove the reg->mem DenseMaps in favor of binary search.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/trunk/lib/Target/X86/X86InstrInfo.cpp:123
+
+static const X86MemoryFoldTableEntry MemoryFoldTable2Addr[] = {
+  { X86::ADC16ri,     X86::ADC16mi,    0 },
----------------
greened wrote:
> Whoa.  So developers are expected to keep these tables sorted, with no comment to that effect and no convenient numeric indication of the opcode values?  This seems like a recipe for mistakes to me.  Instead of asserting that the tables are sorted below, why not just sort them?  Is it too expensive?
Sorting them would require bringing them into a heap allocated vector to sort. Which is a little more costly than leaving them in constant memory.

Conveniently the instruction numeric values are in alphabetical order which is the only reason I really considered this approach. I sorted them by selecting all the lines and doing ":sort" in vim. We also have a tablegen backend that generates a buggy version of the same tables in almost the same order. There are some manual entries hardcoded into tablegen that get dumped at the end of each table that mess up the ordering. I expect new entries to be added by getting them from the auto generated table and bringing them over.

You're absolutely right that I should have left a comment.


Repository:
  rL LLVM

https://reviews.llvm.org/D48527