[PATCH] Replace uint64_t representation of Features with FeatureBitset (std::bitset) in a few more places

Ranjeet Singh ranjeet.singh at arm.com
Tue Jun 23 08:39:23 PDT 2015


In http://reviews.llvm.org/D10542#192727, @mkuper wrote:

> Hm. I've read the patch more carefully, and I'm actually somewhat confused now.
>
> Each target has two enumerations:
>
> 1. SubtargetFeatureFlag  - this is used, I think, only by the AsmParser, and has elements that look like Feature_HasAVX512. This one still lives in a regular bitfield, not a bitset, so the elements of the enum are of them form "1 << C". If I'm not mistaken, ComputeAvailableFeatures() returns a set of these values.
> 2. A nameless enum used everywhere else, which has elements that look like X86::FeatureAVX512. I've changed this enum in my original patch, and now these are ordinal values (1, 2, 3... etc.), instead of powers of 2.
>
>   Now, it looks like what you did was to change everything that used bitfields containing SubtargetFeatureFlag to use a FeatureBitset instead. But the values themselves are still powers of 2, so you are still bound by the limits of the underlying type.
>
>   Is that right? If it is, then what is the goal of this patch?


I missed the fact that the SubtargetFeatureFlags enums are still powers of 2, I should have changed it so that they are ordinal numbers the same way you did it in your original patch because it's possible that the enum list could grow to >64 entries.

Thanks,
Ranjeet


http://reviews.llvm.org/D10542

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list