================
Comment at: lib/Sema/SemaStmtAttr.cpp:126
@@ +125,3 @@
+  // Accumulated state of enable|disable hints for each hint category.
+  bool EnabledIsSet[3] = {false, false, false};
+  int EnabledValue[3];
----------------
Reid Kleckner wrote:
> Rather than having 6 parallel arrays, maybe this would be better as an array 
> of a struct:
>   struct {
>     int EnableOptionId;
>     int NumericOptionId;
>     bool EnabledIsSet;
>     bool ValueIsSet;
>     bool Enabled;
>     int Value;
>   } Options[] = {
>     {LoopHintAttr::Vectorize, LoopHintAttr::VectorizeWidth},
>     {LoopHintAttr::Interleave, LoopHintAttr::InterleaveCount},
>     {LoopHintAttr::Unroll, LoopHintAttr::UnrollCount}
>   };
The [size] on these arrays is superfluous? Also, Reid's suggestion makes sense 
if it ends up simplifying the code

================
Comment at: lib/Sema/SemaStmtAttr.cpp:148
@@ -128,2 +147,3 @@
 
+    int Category;
     switch (Option) {
----------------
Would a local enum {Vectorize = 0, ...} type would make this code cleaner 
instead of magic Category numbers?

================
Comment at: lib/Sema/SemaStmtAttr.cpp:196
@@ -191,1 +195,3 @@
       }
+      if (EnabledIsSet[Category] && EnabledValue[Category] == 0) {
+        // Disable hints are not compatible with numeric hints of the
----------------
Just wondering: could this duplication be avoided if the check was separate and 
not part of setting hte attributes?

http://reviews.llvm.org/D4089



_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to