On Wed, Jun 25, 2014 at 4:28 PM, Mark Heffernan <[email protected]> wrote: > Hi aaron.ballman, eliben, TylerNowicki, > > This patch adds support for the loop unroll pragma syntax defined in the CUDA > c/c++ language: > > // Fully unroll loop. > #pragma unroll > for (...) { } > > // Unroll loop N times. > #pragma unroll N > for (...) { } > > This pragma is only supported when compiling in CUDA mode (-x cuda). The > "#pragma unroll" and "#pragma unroll N" have identical semantics to "#pragma > clang loop unroll(enable)" and "#pragma clang loop unroll_count(N)" > respectively.
I am really uncomfortable with the idea of having two pragmas with identical semantics but differing syntax. I would prefer that we have a single syntax since it is *identical* functionality. I suspect this pragma is driven by the CUDA standard? If so, has the idea been explored of simply supporting CUDA's syntax outside of CUDA mode, and dropping #pramga loop unroll? I don't like how much duplication is happening in this code given that there's already existing machinery in place to do all of this. When other attributes run into this situation, the usual approach is to simply modify a single attribute definition in Attr.td to have a new spelling and the rest of the machinery works as-is. I think this needs to be rationalized with the existing LoopHint attribute functionality so that there is one representation for this under the hood, even if there winds up being two different syntaxes. Eg) there should not be two AST representations for these semantics, there should not be two different sets of "hints" that get created to pass this information around, etc. I have comments about the patch, but until we resolve the "one syntax vs two syntaxes" question, a lot of the comments may be moot. ~Aaron > > Mark > > http://reviews.llvm.org/D4297 > > Files: > docs/ReleaseNotes.rst > include/clang/Basic/Attr.td > include/clang/Basic/AttrDocs.td > include/clang/Basic/DiagnosticSemaKinds.td > include/clang/Basic/TokenKinds.def > include/clang/Parse/Parser.h > include/clang/Sema/CudaUnrollHint.h > lib/CodeGen/CGStmt.cpp > lib/CodeGen/CodeGenFunction.h > lib/Parse/ParsePragma.cpp > lib/Parse/ParseStmt.cpp > lib/Sema/SemaStmtAttr.cpp > test/CodeGen/cuda-pragma-unroll.cu > test/Misc/ast-print-cuda-pragmas.cu > test/PCH/cuda-pragma-unroll.cu > test/Parser/cuda-pragma-unroll.cu _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
