[PATCH] D42334: [NFC] Refactor Apple Accelerator Tables

Pavel Labath via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 24 01:59:32 PST 2018


labath added a comment.

I find the fact that you have to specify type of data in the addName call a bit weird. This means you cannot guarantee the homogeneity of the table, as different addName calls on the same table may use different types (and none of them needs to match the atom array you passed to the constructor). It sounds to me like the data type should really a part of the table type (but you have not done that because of the code size concerns et al.).

This "non-template base class" idea has already been proposed earlier in the thread by Adrian, but got dismissed because at that point you were still trying to optimize everything for speed. Now that you already have a base class for the data types, it should be fairly trivial to implement: put everything except the constructor and the addName function in the base class; the templated constructor can implicitly take the atom array as T::Atoms, and addName will automatically construct entries of type T. That should guarantee consistency with only a tiny code size increase.

What do you think?


https://reviews.llvm.org/D42334





More information about the llvm-commits mailing list