[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