RKSimon added inline comments.
================ Comment at: lib/Basic/Targets.cpp:3189 break; + case CK_ZNVER1: + setFeatureEnabledImpl(Features, "adx", true); ---------------- GGanesh wrote: > RKSimon wrote: > > Same as what I asked on D28017 - is there an accepted order that we should > > be using here? > Some of them seems to be chronological. > Some of them are alphabetical. > > I personally don't have any preference as such. > Alphabetical order suits a long list. > I would like to know your suggestion. @craig.topper Any preferences? No strong preference and nothing that should slow the acceptance of this patch - alphabetical can be easier to maintain but it's unlikely this code changes often. Sorting by feature groups/age can be more understandable, and can help account for the fall-through behaviour used in many of the cases here - speaking of which would it be useful to fall-through from CK_ZNVER1 to CK_BTVER2 to CK_BTVER1 since they seem to have a common set of features? https://reviews.llvm.org/D28018 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits