On Mon, Jun 22, 2020 at 2:09 AM Florian Weimer wrote: > > * H. J. Lu via Libc-alpha: > > > On Thu, Jun 18, 2020 at 1:45 AM Florian Weimer wrote: > >> > >> * H. J. Lu via Libc-alpha: > >> > >> > exports only: > >> > > >> > struct cpu_features > >> > { > >> > struct cpuid_registers cpuid[COMMON_CPUID_INDEX_MAX]; > >> > unsigned int feature[FEATURE_INDEX_MAX]; > >> > struct cpu_features_basic basic; > >> > }; > >> > >> The struct cpu_features ABI does not appear to be stable, as I wrote on > >> the other thread: > >> > >> > >> > > > > Here is the updated patch to use > > > > struct cpu_features > > { > > struct cpu_features_basic basic; > > unsigned int usable[USABLE_FEATURE_INDEX_MAX]; > > struct cpuid_registers cpuid[COMMON_CPUID_INDEX_MAX]; > > }; > > > > This should be backward compatible for both .o and .so files. > > This is an improvement. I still see issues with this interface, though. > > Programmers will need something like this for IFUNC resolvers, but this > is not usable there if the relocation order does not match the explicit > ELF dependency order (e.g., due to symbol interposition). This is a > generic problem with IFUNC resolvers. Other architectures solve this by > adding arguments to IFUNC resolvers, so that the required can be This issue is orthogonal to . > accessed without relocations. __builtin_cpu_supports does not have this > issue. > > The manual does not explain the difference between “available” and > “usable”. I do not think we should expose both. I don't see any > circumstances under which “available” would provide useful information. Fixed. "available" means supported by the operating system, which is very useful information. > struct cpu_features (even in its reduced form) is fairly large. We will > never be able to reduce its size again if it becomes public ABI. Fixed by struct cpu_features { struct cpu_features_basic basic; unsigned int *usable_p; struct cpuid_registers cpuid[COMMON_CPUID_INDEX_MAX]; }; > I'm not convinced that this interface conforms to the API contract for > the CPUID instruction. Aren't the raw bits in the cpuid field subject > to interpretation based on family and model? HAS_CPU_FEATURE does not > reflect this at all. Yes, it does. The cpuid array is populated based on family and model. It has accurate information about processor features. > My preferred interface would look like this: > > * The low-level function call ABI is: > > unsigned long long long int __x86_get_cpu_features (unsigned int word) > __attribute__ ((const)); > > * IFUNC resolvers receive two arguments, > > unsigned long long long int *feature_words, size_t feature_words_length > > * CPU_FEATURE_USABLE (NAME) would expand to something like > > ((__x86_get_cpu_features (NAME_word) & NAME_bitmask) != 0) > > The implementation of __x86_get_cpu_features simply returns 0 if the > index is too large, which solves the extension problem. > > * CPU_FEATURE_USABLE_ARRAY (WORDS, WORDS_LENGTH, NAME) would expand to this: > > (((NAME_word < WORDS_LENGTH ? WORDS[NAME_word] ? : 0) & NAME_bitmask) != 0) > > This version is expected to be used in IFUNC resolvers. The macro > name is just a strawman. > > Only the “usable” bits are exposed, so the interface makes it clear that > the implementation performs some normalization. My new interface is expandable and backward compatible. It is also forward compatible since the older libc.so works with binaries compiled against the newer as long as the cpuid and usable array sizes are the same. > Alternatively, the __builtin_cpu_supports interface could be enhanced to > cover all the features you need. __builtin_cpu_supports is equivalent to CPU_FEATURE_USABLE and it doesn't support HAS_CPU_FEATURE which does provide useful information. -- H.J.