aaron.ballman added inline comments.
================ Comment at: include/clang/Basic/AttrDocs.td:2582 specified for the subsequent loop. The directive allows vectorization, -interleaving, and unrolling to be enabled or disabled. Vector width as well -as interleave and unrolling count can be manually specified. See -`language extensions +interleaving, and unrolling to be enabled or disabled. Pipelining could be disabled. +Vector width, interleave count, unrolling count, and the initiation interval for pipelining ---------------- alexey.lapshin wrote: > aaron.ballman wrote: > > Can you combine the new sentence with the previous one? `The directive > > allows vectorization, interleaving, pipelining, and unrolling to be enabled > > or disabled.` > In that case it would change a meaning and become incorrect. Pipelining could > only be disabled. It could not be enabled. Ah, yes, that would change the meaning then. I'd go with: `The directive allows pipelining to be disabled, or vectorization, interleaving, and unrolling to be enabled or disabled.` ================ Comment at: include/clang/Basic/AttrDocs.td:2663 + could be used as hints for the software pipelining optimization. The pragma is + placed immediately before a for, while, do-while, or a C++11 range-based for + loop. ---------------- I'd like to see a Sema test where the pragma is put at global scope to see if the diagnostic is reasonable or not. e.g., ``` #pragma clang loop pipeline(disable) int main() { for (int i = 0; i < 10; ++i) ; } ``` ================ Comment at: include/clang/Basic/AttrDocs.td:2680-2681 + the specified cycle count. If a schedule was not found then loop + remains unchanged. The initiation interval must be a positive number + greater than zero: + ---------------- Please add a Sema test with a negative number as the initiation interval. ================ Comment at: include/clang/Basic/DiagnosticParseKinds.td:1188 + "vectorize_width, interleave, interleave_count, unroll, unroll_count, " + "pipeline, pipeline_initiation_interval or distribute">; ---------------- pipeline_initiation_interval or distribute -> pipeline_initiation_interval, or distribute (Keeps the Oxford comma as in the original diagnostic.) ================ Comment at: test/Parser/pragma-pipeline.cpp:28 +/* expected-error {{invalid argument of type 'double'; expected an integer type}} */ #pragma clang loop pipeline_initiation_interval(1.0) +/* expected-error {{invalid value '0'; must be positive}} */ #pragma clang loop pipeline_initiation_interval(0) + for (int i = 0; i < Length; i++) { ---------------- This isn't really a parser test -- it should probably be in Sema instead. ================ Comment at: test/Parser/pragma-pipeline.cpp:35-47 +#pragma clang loop pipeline(disable) +/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int j = Length; +#pragma clang loop pipeline_initiation_interval(4) +/* expected-error {{expected a for, while, or do-while loop to follow '#pragma clang loop'}} */ int k = Length; + +#pragma clang loop pipeline(disable) +#pragma clang loop pipeline_initiation_interval(4) /* expected-error {{incompatible directives 'pipeline(disable)' and 'pipeline_initiation_interval(4)'}} */ ---------------- These should also move to Sema. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55710/new/ https://reviews.llvm.org/D55710 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits