[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