[PATCH] Add loop unroll pragma support

Eli Bendersky eliben at google.com
Tue Jun 10 15:55:21 PDT 2014


================
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






More information about the cfe-commits mailing list