Changes to MCInstrDesc and tablegenned instruction tables (was Re: Impact of default PIE on llvm testing times)

As discussed here I noticed that when my host toolchain enabled PIE by default, the time to run check-llvm increased by about 1.75x. Much of this is due to tables of readonly data containing pointers, which with PIE need to be fixed up by ld.so when the executable (e.g. llc) is loaded.

Some of the biggest offenders are the tables of MCInstrDesc for each instruction, which are autogenerated by tablegen and contain pointers to lists of implicit operands and to lists of MCOperandInfo for each explicit operand. I have patches to fix this by changing MCInstrDesc to use offsets instead of pointers to find these lists:

https://reviews.llvm.org/D142218
https://reviews.llvm.org/D142219

On my machine this reduces the time taken for a Release build check-llvm from 204 seconds to 131 seconds with PIE enabled. With PIE disabled it takes 113 seconds. I assume it would also reduce the time taken to load libLLVM.so but I have not tried to measure this.

This is a pretty significant change to MCInstrDesc so @s-barannikov suggested I raise it here for awareness. Does anyone have any concerns? Or feel inclined to add their approval to the patches?

I have no concerns, except that this post indicates that users of the MCInstrDesc interface need to be aware of its internals. If we can take away that necessity, I think any improvement in dynamic linktime is an unconditional win. (BTW, I think that static link time will also be smaller.)
I think link time could further be reduced by wrapping a base address pointer of the referenced tables with the referring table into a structure.

MCInstrDesc contains fields, which will change, and accessor methods, which won’t. Everything is public so there is nothing to stop you using the fields directly, but I have already landed some preparatory patches which change all in-tree code to use the accessor methods instead. Hopefully it will be easy for out-of-tree code to do the same, so they won’t be affected if/when the last two patches land.

Conceptually the big change is that an MCInstrDesc will start using offsets from its own address to find information about its operands. This means that you can’t copy them (because the offsets won’t be correct for the new address) and an MCInstrDesc has to live in the same allocation as its operand lists (so the offsets are guaranteed to be smallish). In-tree code generally only uses the MCInstrDescs that live in the big table of instructions autogenerated by tablegen, so there is nothing to worry about. Conceivably there could be some out-of-tree code that does something weirder, like dynamically allocating new MCInstrDescs, in which case it would need to be aware of the new constraints.

I guess the questions for the community are:

  1. Are you generally happy with this design change? E.g. is it going to conflict with any other plans you have for MCInstrDesc?
  2. Are you aware of any out-of-tree code for which this might cause big problems?

Funny you would mention this. I’ve recently tried to dynamically tie an itinerary to an instruction. It would have been nice if there would have been an overridable method somewhere, although this was at MachineInstr level.
I think getting rid of these public internals is a very good step. Carry on!