Hi Aaron, Thanks for the review! Here is the updated patch and responses to some of your comments. The new tests are a couple of lines added to test/Parser/pragma-loop.cpp. This patch is just refactoring in preparation for new features so there really isn’t a need for new tests.
I’m pretty sure Default or something like it is correct. This value is used for #pragma unroll and #pragma nounroll. The default behavior with unroll enables the unroller and nounroll disables it. UsesValue would imply that examining the state is required to determine if the Value parameter is used. What I would like to see in the future is for the state and value to be unioned. And the Option would determine if the state or value was needed. AFAIK Tablegen doesn’t have the ability to express unions right now. + return true; The naming of the pragmas is unfortunately a little confusing. #pragma unroll and #pragma nounroll don’t have options. There is no need because they only control unrolling. On the other hand #pragma clang loop has options which can be vectorize, interleave, unroll, etc. So when pragma unroll is used we don’t do the check that determines if the option has an argument for the state or value of the loop hint. #pragma unroll -> enables unrolling #pragma nounroll -> disables unrolling #pragma unroll(…) or #pragma unroll … -> specifies the amount of unrolling The first 2 cases are handled by the early test: if (!Info->HasValue && (PragmaUnroll || PragmaNoUnroll)) { … return true; } After that we can always assume the 3rd case. So we leave StateOption false and drop down to the part that assumes the argument is a value. Tyler |
loop_hint_attr_state-svn.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
