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

Reply via email to