[PATCH] #pragma vectorize
Ben Langmuir
blangmuir at apple.com
Mon May 12 08:16:25 PDT 2014
> Index: test/CodeGen/pragma-loop.cpp
> ===================================================================
> --- test/CodeGen/pragma-loop.cpp (revision 0)
> +++ test/CodeGen/pragma-loop.cpp (working copy)
> @@ -0,0 +1,120 @@
> +// RUN: %clang -std=c++11 -emit-llvm -S -o - %s | FileCheck %s
%clang_cc1 ?
On May 12, 2014, at 7:07 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
> More comments in the review, as well as some replies to previous email.
>
> On Fri, May 9, 2014 at 3:59 PM, Tyler Nowicki <tnowicki at apple.com> wrote:
>> Hi Aaron, Richard, Ben,
>>
>> Thanks for all the feedback. I’ve tried to integrate all of your suggested.
>> I may have missed some, please let me know if I did.
>>
>> Please review the attached patch.
>>
>> Tyler
>>
>>
>>
>>
>> + // Add vectorize hints to the metadata on the conditional branch.
>> + // Iterate in reverse so hints are put in the same order they appear.
>> + SmallVector<llvm::Value*, 2> Metadata(1);
>> + ArrayRef<const Attr*>::reverse_iterator I;
>> + for (I = Attrs.rbegin(); I != Attrs.rend(); ++I) {
>>
>>
>> Check against rend() should be hoisted (or use a range-based loop with
>> an adaptor).
>>
>>
>> Sorry, I don’t understand what you are suggesting. Do we not compare against
>> rend() in the loop condition?
>
> We do, but it is more idiomatic in the codebase to hoist the call so
> it's not made on every loop iteration. Eg)
>
> for (auto I = Attrs.rbegin(), E = Attrs.rend(); I != E; ++I)
>
>> I’m pretty sure a spelling is required unless you are providing the base
>> class outside the td file. I’m looking at modifying the way pretty printing
>> works now to generate a printer that prints the pragma. But it seems bad to
>> specialize the code that generates the pretty printer for printing loop
>> pragmas
>>
>>
>> Why?
>>
>>
>> That part of tablegen probably shouldn’t be specialized for each type of
>> pragma should it? But from your comment below it sounds like thats what you
>> are thinking.
>
> I'd have to think about it more, but it seems like tablegen shouldn't
> have to specialize for each pragma, just all pragmas. Eg) the
> difference between printing pragmas and printing attributes is minor
> enough that it could be handled entirely by tablegen without the
> pragma authors having to write special code.
>
>> Perhaps we should provide a separate method for printing pragmas rather than
>> using printPretty. Right now printPretty returns the attribute's text and
>> StmtPrinter::VisitAttributeStmt puts that between ‘[[‘ and ‘]]’. We need
>> some way of putting the pragmas prior to the attribute. We could first loop
>> over all attributes calling printPragma, and then again using printPretty to
>> generate the attributes. Where printPragma would generate directives like
>> #pragma loop ...
>
> Yes, we would have to separate things out a bit more.
>
>>
>> and so it will still be necessary to make some kind of a call into
>> the LoopHintAttr to print the pragma.
>>
>>
>> So what I was envisioning was that there would be a Pragma spelling
>> added to Attr.td, which ClangAttrEmitter.cpp could then key off of for
>> everything which is spelling-specific (you can search for CXX11 or
>> Declspec in the file and that should show you the places that may need
>> modification). However, that's orthogonal to your functionality, and
>> not really something you should have to implement yourself just to get
>> this patch in. I just don't have the time to tackle it myself right
>> now.
>>
>>
>> Sounds good. I started looking at this I have a basic patch for Pragma
>> spelling. After #pragma loop I’ll look at that again.
>>
>>
>> So I guess what I am saying is: once there's a new patch that
>> addresses most of the concerns (aside from stuff like modifying the
>> way the attr tablegen works), we can see if there's anything else
>> blocking your patch. Don't worry about modifying tablegen for this,
>> that's more a FIXME for the future.
>
>> Index: include/clang/AST/Attr.h
>> ===================================================================
>> --- include/clang/AST/Attr.h (revision 208442)
>> +++ include/clang/AST/Attr.h (working copy)
>> @@ -16,6 +16,7 @@
>>
>> #include "clang/AST/AttrIterator.h"
>> #include "clang/AST/Decl.h"
>> +#include "clang/AST/Expr.h"
>> #include "clang/AST/Type.h"
>> #include "clang/Basic/AttrKinds.h"
>> #include "clang/Basic/LLVM.h"
>> Index: include/clang/Basic/Attr.td
>> ===================================================================
>> --- include/clang/Basic/Attr.td (revision 208442)
>> +++ include/clang/Basic/Attr.td (working copy)
>> @@ -1750,3 +1750,65 @@
>> def Unaligned : IgnoredAttr {
>> let Spellings = [Keyword<"__unaligned">];
>> }
>> +
>> +def LoopHint : Attr {
>> + // LoopHint Vectorize:
>> + // enable - use vector instructions.
>> + // disable - do not use vector instructions.
>> + // positive value - use vector instructions of the specified width.
>> +
>> + // LoopHint Interleave:
>> + // enable - interleave multiple loop iterations.
>> + // disable - do not interleave multiple loop interactions.
>> + // positive value - interleave the specified number of loop interations.
>> +
>> + // FIXME: Add Pragma spelling to tablegen and
>> + // use it here.
>> + let Spellings = [Keyword<"loop">];
>> +
>> + // State of the loop optimization specified by the spelling.
>> + let Args = [EnumArgument<"Option", "OptionType",
>> + ["vectorize", "interleave"],
>> + ["Vectorize", "Interleave"]>,
>> + EnumArgument<"Kind","KindType",
>
> Missing a space after the comma.
>
>> + ["disable", "enable", "value"],
>> + ["Disable", "Enable", "Value"]>,
>
> This is actually an optional argument as well, but is not marked as
> such. It should get a , 1. Also, this suggests we need a new argument
> type that represents a union of arguments, since that's really what
> you want (one of these two arguments must be used, but you don't care
> which). A FIXME would probably be appropriate (though you don't have
> to implement the functionality for this patch).
>
>> + ExprArgument<"Value", 1>];
>
> Judging by the tests, this should be a DefaultIntArgument<"Value", 1>.
> Either that, or there are tests missing where expressions are used
> (and honestly, it would strike me as slightly strange to allow general
> expressions here).
>
>> +
>> + let AdditionalMembers = [{
>> + // Kinds are compatible if they are not exclusive.
>> + static bool isCompatible(int Kind1, int Kind2) {
>> + return (Kind1==Disable) == (Kind2==Disable);
>
> Missing spaces around the inner ==.
>
>> + }
>> +
>> + static StringRef getOptionName(int Option) {
>> + switch (Option) {
>> + case Vectorize: return "vectorize";
>> + case Interleave: return "interleave";
>> + }
>> + return "unknown";
>
> Seems like this should be an llvm_unreachable instead of returning unknown.
>
>> + }
>> +
>> + static StringRef getKindName(int Kind) {
>> + switch (Kind) {
>> + case Disable: return "disable";
>> + case Enable: return "enable";
>> + case Value: return "value";
>> + }
>> + return "unknown";
>
> Same here.
>
>> + }
>> +
>> + // FIXME: Modify pretty printer to print this pragma
>> + void print(raw_ostream &OS, const PrintingPolicy &Policy) const {
>> + OS << getOptionName(option) << "(";
>> + if (getKind() == Value) {
>> + getValue()->printPretty(OS, 0, Policy);
>> + } else {
>> + OS << getKindName(getKind());
>> + }
>
> No curly braces on single-line if/else statements.
>
>> + OS << ")\n";
>> + }
>> + }];
>> +
>> + let Documentation = [Undocumented];
>> +}
>> Index: include/clang/Basic/DiagnosticParseKinds.td
>> ===================================================================
>> --- include/clang/Basic/DiagnosticParseKinds.td (revision 208442)
>> +++ include/clang/Basic/DiagnosticParseKinds.td (working copy)
>> @@ -884,6 +884,12 @@
>> "unexpected OpenMP clause '%0' in directive '#pragma omp %1'">;
>> def err_omp_more_one_clause : Error<
>> "directive '#pragma omp %0' cannot contain more than one '%1' clause">;
>> +
>> +// Pragma loop support.
>> +def err_pragma_loop_invalid_option : Error<
>> + "%select{invalid|missing}0 option%select{ '%1'|}0 in directive '#pragma loop'; expected either vectorize or interleave">;
>
> 80-col limit.
>
>> +def err_pragma_loop_invalid_type : Error<
>> + "invalid value '%0' in directive '#pragma loop %1'; expected either 'enable', 'disable', or a positive integer">;
>
> Same here.
>
>> } // end of Parse Issue category.
>>
>> let CategoryName = "Modules Issue" in {
>> Index: include/clang/Basic/DiagnosticSemaKinds.td
>> ===================================================================
>> --- include/clang/Basic/DiagnosticSemaKinds.td (revision 208442)
>> +++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
>> @@ -537,6 +537,12 @@
>> "#pragma visibility pop with no matching #pragma visibility push">;
>> def note_surrounding_namespace_starts_here : Note<
>> "surrounding namespace with visibility attribute starts here">;
>> +def err_pragma_loop_invalid_value : Error<
>> + "expected a positive integer in directive '#pragma loop %0'">;
>> +def err_pragma_loop_incompatible : Error<
>> + "'%0' and '%1' directive option types are incompatible in '#pragma loop %2'">;
>> +def err_pragma_loop_precedes_nonloop : Error<
>> + "expected a for, while, or do-while loop to follow the '#pragma loop' directive">;
>
> 80-col limit.
>
>>
>> /// Objective-C parser diagnostics
>> def err_duplicate_class_def : Error<
>> Index: include/clang/Basic/TokenKinds.def
>> ===================================================================
>> --- include/clang/Basic/TokenKinds.def (revision 208442)
>> +++ include/clang/Basic/TokenKinds.def (working copy)
>> @@ -701,6 +701,11 @@
>> ANNOTATION(pragma_openmp)
>> ANNOTATION(pragma_openmp_end)
>>
>> +// Annotations for loop pragma directives #pragma loop ...
>> +// The lexer produces these so that they only take effect when the parser
>> +// handles #pragma loop ... directives.
>> +ANNOTATION(pragma_loop_hint)
>> +
>> // Annotations for module import translated from #include etc.
>> ANNOTATION(module_include)
>> ANNOTATION(module_begin)
>> Index: include/clang/Parse/Parser.h
>> ===================================================================
>> --- include/clang/Parse/Parser.h (revision 208442)
>> +++ include/clang/Parse/Parser.h (working copy)
>> @@ -20,6 +20,7 @@
>> #include "clang/Lex/CodeCompletionHandler.h"
>> #include "clang/Lex/Preprocessor.h"
>> #include "clang/Sema/DeclSpec.h"
>> +#include "clang/Sema/LoopHint.h"
>> #include "clang/Sema/Sema.h"
>> #include "llvm/ADT/SmallVector.h"
>> #include "llvm/Support/Compiler.h"
>> @@ -160,6 +161,7 @@
>> std::unique_ptr<PragmaHandler> MSConstSeg;
>> std::unique_ptr<PragmaHandler> MSCodeSeg;
>> std::unique_ptr<PragmaHandler> MSSection;
>> + std::unique_ptr<PragmaHandler> LoopHintHandler;
>>
>> std::unique_ptr<CommentHandler> CommentSemaHandler;
>>
>> @@ -518,6 +520,10 @@
>> /// #pragma clang __debug captured
>> StmtResult HandlePragmaCaptured();
>>
>> + /// \brief Handle the annotation token produced for
>> + /// #pragma vectorize...
>> + LoopHint HandlePragmaLoopHint();
>> +
>> /// GetLookAheadToken - This peeks ahead N tokens and returns that token
>> /// without consuming any tokens. LookAhead(0) returns 'Tok', LookAhead(1)
>> /// returns the token after Tok, etc.
>> @@ -1600,6 +1606,10 @@
>> StmtResult ParseReturnStatement();
>> StmtResult ParseAsmStatement(bool &msAsm);
>> StmtResult ParseMicrosoftAsmStatement(SourceLocation AsmLoc);
>> + StmtResult ParsePragmaLoopHint(StmtVector &Stmts,
>> + bool OnlyStatement,
>> + SourceLocation *TrailingElseLoc,
>> + ParsedAttributesWithRange &Attrs);
>>
>> /// \brief Describes the behavior that should be taken for an __if_exists
>> /// block.
>> Index: include/clang/Sema/LoopHint.h
>> ===================================================================
>> --- include/clang/Sema/LoopHint.h (revision 0)
>> +++ include/clang/Sema/LoopHint.h (working copy)
>> @@ -0,0 +1,32 @@
>> +//===--- LoopHint.h - Types for LoopHint ------------------------*- C++ -*-===//
>> +//
>> +// The LLVM Compiler Infrastructure
>> +//
>> +// This file is distributed under the University of Illinois Open Source
>> +// License. See LICENSE.TXT for details.
>> +//
>> +//===----------------------------------------------------------------------===//
>> +
>> +
>> +#ifndef LLVM_CLANG_SEMA_LOOPHINT_H
>> +#define LLVM_CLANG_SEMA_LOOPHINT_H
>> +
>> +#include "clang/Basic/IdentifierTable.h"
>> +#include "clang/Basic/SourceLocation.h"
>> +#include "clang/Sema/AttributeList.h"
>> +#include "clang/Sema/Ownership.h"
>> +
>> +namespace clang {
>> +
>> +/// \brief Loop hint specified by a pragma loop directive.
>> +struct LoopHint {
>> + SourceRange Range;
>> + Expr* ValueExpr;
>
> Asterisk on the right, instead of the left.
>
>> + IdentifierLoc *LoopLoc;
>> + IdentifierLoc *ValueLoc;
>> + IdentifierLoc *OptionLoc;
>> +};
>> +
>> +} // end namespace clang
>> +
>> +#endif // LLVM_CLANG_SEMA_LOOPHINT_H
>> Index: lib/AST/StmtPrinter.cpp
>> ===================================================================
>> --- lib/AST/StmtPrinter.cpp (revision 208442)
>> +++ lib/AST/StmtPrinter.cpp (working copy)
>> @@ -168,19 +168,34 @@
>> }
>>
>> void StmtPrinter::VisitAttributedStmt(AttributedStmt *Node) {
>> - OS << "[[";
>> bool first = true;
>> - for (ArrayRef<const Attr*>::iterator it = Node->getAttrs().begin(),
>> - end = Node->getAttrs().end();
>> - it != end; ++it) {
>> - if (!first) {
>> - OS << ", ";
>> - first = false;
>> + std::string raw_attr_os;
>> + llvm::raw_string_ostream AttrOS(raw_attr_os);
>> + for (ArrayRef<const Attr*>::reverse_iterator it = Node->getAttrs().rbegin(),
>> + end = Node->getAttrs().rend();
>> + it != end; ++it) {
>
> Good time to update this to use "I" instead of "it" (similar for
> "end"). Also, (I apologize if this was covered elsewhere) why did this
> code switch from using begin/end to rbegin/rend?
>
>> + // FIXME: This hack will be removed when printPretty
>> + // has been modified to print pretty pragmas
>
> Missing a period.
>
>> + if (dyn_cast<LoopHintAttr>(*it)) {
>> + const LoopHintAttr *LHA = cast<LoopHintAttr>(*it);
>> + OS << "#pragma loop ";
>> + LHA->print(OS, Policy);
>> + } else {
>> + if (!first) {
>> + AttrOS << ", ";
>> + first = false;
>> + }
>> + // TODO: check this
>> + (*it)->printPretty(AttrOS, Policy);
>> }
>> - // TODO: check this
>> - (*it)->printPretty(OS, Policy);
>> }
>> - OS << "]] ";
>> +
>> + // Check to see if any attributes were printed
>> + StringRef AttrStr = AttrOS.str();
>> + if (!AttrStr.empty()) {
>> + OS << "[[" << AttrStr << "]] ";
>> + }
>
> Should elide the curly braces.
>
>> +
>> PrintStmt(Node->getSubStmt(), 0);
>> }
>>
>> Index: lib/CodeGen/CGStmt.cpp
>> ===================================================================
>> --- lib/CodeGen/CGStmt.cpp (revision 208442)
>> +++ lib/CodeGen/CGStmt.cpp (working copy)
>> @@ -18,6 +18,7 @@
>> #include "clang/AST/StmtVisitor.h"
>> #include "clang/Basic/PrettyStackTrace.h"
>> #include "clang/Basic/TargetInfo.h"
>> +#include "clang/Sema/LoopHint.h"
>> #include "clang/Sema/SemaDiagnostic.h"
>> #include "llvm/ADT/StringExtras.h"
>> #include "llvm/IR/CallSite.h"
>> @@ -396,7 +397,23 @@
>> }
>>
>> void CodeGenFunction::EmitAttributedStmt(const AttributedStmt &S) {
>> - EmitStmt(S.getSubStmt());
>> + const Stmt *SubStmt = S.getSubStmt();
>> + switch (SubStmt->getStmtClass()) {
>> + case Stmt::DoStmtClass:
>> + EmitDoStmt(cast<DoStmt>(*SubStmt), S.getAttrs());
>> + break;
>> + case Stmt::ForStmtClass:
>> + EmitForStmt(cast<ForStmt>(*SubStmt), S.getAttrs());
>> + break;
>> + case Stmt::WhileStmtClass:
>> + EmitWhileStmt(cast<WhileStmt>(*SubStmt), S.getAttrs());
>> + break;
>> + case Stmt::CXXForRangeStmtClass:
>> + EmitCXXForRangeStmt(cast<CXXForRangeStmt>(*SubStmt), S.getAttrs());
>> + break;
>> + default:
>> + EmitStmt(SubStmt);
>> + }
>> }
>>
>> void CodeGenFunction::EmitGotoStmt(const GotoStmt &S) {
>> @@ -502,7 +519,86 @@
>> EmitBlock(ContBlock, true);
>> }
>>
>> -void CodeGenFunction::EmitWhileStmt(const WhileStmt &S) {
>> +void CodeGenFunction::EmitCondBrHints(llvm::LLVMContext &Context,
>> + llvm::BranchInst *CondBr,
>> + ArrayRef<const Attr*> &Attrs) {
>> + // Do not continue if there are not hints.
>> + if (Attrs.empty())
>> + return;
>> +
>> + // Add vectorize hints to the metadata on the conditional branch.
>> + // Iterate in reverse so hints are put in the same order they appear.
>> + SmallVector<llvm::Value*, 2> Metadata(1);
>> + ArrayRef<const Attr*>::reverse_iterator I;
>
> No need for this to live outside of the for loop. Should be scoped to
> the loop, and you can use auto if length is worrisome.
>
>> + for (I = Attrs.rbegin(); I != Attrs.rend(); ++I) {
>> + const LoopHintAttr *LH = dyn_cast<LoopHintAttr>(*I);
>> +
>> + // Skip non loop hint attributes
>> + if (!LH)
>> + continue;
>> +
>> + LoopHintAttr::OptionType Option = LH->getOption();
>> + Expr *ValueExpr = LH->getValue();
>> + int Kind = LH->getKind();
>> +
>> + int ValueInt = 1;
>> + if (Kind == LoopHintAttr::Value) {
>> + llvm::APSInt ValueAPS =
>> + ValueExpr->EvaluateKnownConstInt(CGM.getContext());
>> + ValueInt = ValueAPS.getSExtValue();
>> + }
>> +
>> + llvm::Value *Value;
>> + llvm::MDString *Name;
>> + if (Option == LoopHintAttr::Vectorize) {
>> + switch(Kind) {
>> + case LoopHintAttr::Disable:
>> + Name = llvm::MDString::get(Context, "llvm.vectorizer.width");
>> + Value = llvm::ConstantInt::get(Int32Ty, 1);
>> + break;
>> + case LoopHintAttr::Enable:
>> + Name = llvm::MDString::get(Context, "llvm.vectorizer.enable");
>> + Value = Builder.getTrue();
>> + break;
>> + case LoopHintAttr::Value:
>> + Name = llvm::MDString::get(Context, "llvm.vectorizer.width");
>> + Value = llvm::ConstantInt::get(Int32Ty, ValueInt);
>> + break;
>> + }
>> + } else if (Option == LoopHintAttr::Interleave) {
>> + switch(Kind) {
>> + case LoopHintAttr::Disable:
>> + Name = llvm::MDString::get(Context, "llvm.vectorizer.unroll");
>> + Value = llvm::ConstantInt::get(Int32Ty, 1);
>> + break;
>> + case LoopHintAttr::Enable:
>> + Name = llvm::MDString::get(Context, "llvm.vectorizer.enable");
>> + Value = Builder.getTrue();
>> + break;
>
> This is identical to the vectorize case; is that expected?
>
>> + case LoopHintAttr::Value:
>> + Name = llvm::MDString::get(Context, "llvm.vectorizer.unroll");
>> + Value = llvm::ConstantInt::get(Int32Ty, ValueInt);
>> + break;
>> + }
>> + }
>
> It seems as though the values are identical, regardless of the option,
> and the names are a simple mapping. Perhaps this could be more clearly
> expressed as:
>
> const char *Names[] = { "llvm.vectorizer.width", "llvm.vectorizer.unroll" };
> llvm::Value *Value;
> llvm::MDString *Name;
>
> if (Kind == LoopHintAttr::Enable) {
> Name = llvm::MDString::get(Context, "llvm.vectorizer.enable");
> Value = Builder.getTrue();
> } else {
> Name = llvm::MDString::get(Context, Names[Option]);
> Value = llvm::ConstantInt::get(Int32Ty, ValueInt); // You already
> set ValueInt to 1 by default, and overwrite when the Kind is a Value.
> }
>
>> +
>> + SmallVector<llvm::Value*, 2> OpValues;
>> + OpValues.push_back(Name);
>> + OpValues.push_back(Value);
>> +
>> + // Set or overwrite metadata indicated by Name.
>> + Metadata.push_back(llvm::MDNode::get(Context, OpValues));
>> + }
>> +
>> + // Add llvm.loop MDNode to CondBr.
>> + llvm::MDNode *LoopID = llvm::MDNode::get(Context, Metadata);
>> + LoopID->replaceOperandWith(0, LoopID); // First op points to itself.
>> +
>> + CondBr->setMetadata("llvm.loop", LoopID);
>> +}
>> +
>> +void CodeGenFunction::EmitWhileStmt(const WhileStmt &S,
>> + ArrayRef<const Attr*> WhileAttrs) {
>> RegionCounter Cnt = getPGORegionCounter(&S);
>>
>> // Emit the header for the loop, which will also become
>> @@ -547,13 +643,17 @@
>> llvm::BasicBlock *ExitBlock = LoopExit.getBlock();
>> if (ConditionScope.requiresCleanups())
>> ExitBlock = createBasicBlock("while.exit");
>> - Builder.CreateCondBr(BoolCondVal, LoopBody, ExitBlock,
>> - PGO.createLoopWeights(S.getCond(), Cnt));
>> + llvm::BranchInst *CondBr = Builder.CreateCondBr(
>> + BoolCondVal, LoopBody, ExitBlock,
>> + PGO.createLoopWeights(S.getCond(), Cnt));
>>
>> if (ExitBlock != LoopExit.getBlock()) {
>> EmitBlock(ExitBlock);
>> EmitBranchThroughCleanup(LoopExit);
>> }
>> +
>> + // Attach metadata to loop body conditional branch.
>> + EmitCondBrHints(LoopBody->getContext(), CondBr, WhileAttrs);
>> }
>>
>> // Emit the loop body. We have to emit this in a cleanup scope
>> @@ -582,7 +682,8 @@
>> SimplifyForwardingBlocks(LoopHeader.getBlock());
>> }
>>
>> -void CodeGenFunction::EmitDoStmt(const DoStmt &S) {
>> +void CodeGenFunction::EmitDoStmt(const DoStmt &S,
>> + ArrayRef<const Attr*> DoAttrs) {
>> JumpDest LoopExit = getJumpDestInCurrentScope("do.end");
>> JumpDest LoopCond = getJumpDestInCurrentScope("do.cond");
>>
>> @@ -619,10 +720,15 @@
>> EmitBoolCondBranch = false;
>>
>> // As long as the condition is true, iterate the loop.
>> - if (EmitBoolCondBranch)
>> - Builder.CreateCondBr(BoolCondVal, LoopBody, LoopExit.getBlock(),
>> - PGO.createLoopWeights(S.getCond(), Cnt));
>> + if (EmitBoolCondBranch) {
>> + llvm::BranchInst *CondBr = Builder.CreateCondBr(
>> + BoolCondVal, LoopBody, LoopExit.getBlock(),
>> + PGO.createLoopWeights(S.getCond(), Cnt));
>>
>> + // Attach metadata to loop body conditional branch.
>> + EmitCondBrHints(LoopBody->getContext(), CondBr, DoAttrs);
>> + }
>> +
>> // Emit the exit block.
>> EmitBlock(LoopExit.getBlock());
>>
>> @@ -632,7 +738,8 @@
>> SimplifyForwardingBlocks(LoopCond.getBlock());
>> }
>>
>> -void CodeGenFunction::EmitForStmt(const ForStmt &S) {
>> +void CodeGenFunction::EmitForStmt(const ForStmt &S,
>> + ArrayRef<const Attr*> ForAttrs) {
>> JumpDest LoopExit = getJumpDestInCurrentScope("for.end");
>>
>> RunCleanupsScope ForScope(*this);
>> @@ -686,9 +793,13 @@
>> // C99 6.8.5p2/p4: The first substatement is executed if the expression
>> // compares unequal to 0. The condition must be a scalar type.
>> llvm::Value *BoolCondVal = EvaluateExprAsBool(S.getCond());
>> - Builder.CreateCondBr(BoolCondVal, ForBody, ExitBlock,
>> - PGO.createLoopWeights(S.getCond(), Cnt));
>> + llvm::BranchInst *CondBr = Builder.CreateCondBr(
>> + BoolCondVal, ForBody, ExitBlock,
>> + PGO.createLoopWeights(S.getCond(), Cnt));
>>
>> + // Attach metadata to loop body conditional branch.
>> + EmitCondBrHints(ForBody->getContext(), CondBr, ForAttrs);
>> +
>> if (ExitBlock != LoopExit.getBlock()) {
>> EmitBlock(ExitBlock);
>> EmitBranchThroughCleanup(LoopExit);
>> @@ -728,7 +839,8 @@
>> EmitBlock(LoopExit.getBlock(), true);
>> }
>>
>> -void CodeGenFunction::EmitCXXForRangeStmt(const CXXForRangeStmt &S) {
>> +void CodeGenFunction::EmitCXXForRangeStmt(const CXXForRangeStmt &S,
>> + ArrayRef<const Attr*> ForAttrs) {
>> JumpDest LoopExit = getJumpDestInCurrentScope("for.end");
>>
>> RunCleanupsScope ForScope(*this);
>> @@ -761,9 +873,13 @@
>> // The body is executed if the expression, contextually converted
>> // to bool, is true.
>> llvm::Value *BoolCondVal = EvaluateExprAsBool(S.getCond());
>> - Builder.CreateCondBr(BoolCondVal, ForBody, ExitBlock,
>> - PGO.createLoopWeights(S.getCond(), Cnt));
>> + llvm::BranchInst *CondBr = Builder.CreateCondBr(BoolCondVal, ForBody,
>> + ExitBlock,
>> + PGO.createLoopWeights(S.getCond(), Cnt));
>>
>> + // Attach metadata to loop body conditional branch
>> + EmitCondBrHints(ForBody->getContext(), CondBr, ForAttrs);
>> +
>> if (ExitBlock != LoopExit.getBlock()) {
>> EmitBlock(ExitBlock);
>> EmitBranchThroughCleanup(LoopExit);
>> Index: lib/CodeGen/CodeGenFunction.h
>> ===================================================================
>> --- lib/CodeGen/CodeGenFunction.h (revision 208442)
>> +++ lib/CodeGen/CodeGenFunction.h (working copy)
>> @@ -1846,9 +1846,16 @@
>> void EmitGotoStmt(const GotoStmt &S);
>> void EmitIndirectGotoStmt(const IndirectGotoStmt &S);
>> void EmitIfStmt(const IfStmt &S);
>> - void EmitWhileStmt(const WhileStmt &S);
>> - void EmitDoStmt(const DoStmt &S);
>> - void EmitForStmt(const ForStmt &S);
>> +
>> + void EmitCondBrHints(llvm::LLVMContext &Context,
>> + llvm::BranchInst *CondBr,
>> + ArrayRef<const Attr*> &Attrs);
>> + void EmitWhileStmt(const WhileStmt &S,
>> + ArrayRef<const Attr*> Attrs = ArrayRef<const Attr*>());
>> + void EmitDoStmt(const DoStmt &S,
>> + ArrayRef<const Attr*> Attrs = ArrayRef<const Attr*>());
>> + void EmitForStmt(const ForStmt &S,
>> + ArrayRef<const Attr*> Attrs = ArrayRef<const Attr*>());
>
> In all cases, ArrayRef<const Attr*> is missing a space between the
> identifier and the asterisk.
>
>> void EmitReturnStmt(const ReturnStmt &S);
>> void EmitDeclStmt(const DeclStmt &S);
>> void EmitBreakStmt(const BreakStmt &S);
>> @@ -1872,7 +1879,8 @@
>>
>> void EmitCXXTryStmt(const CXXTryStmt &S);
>> void EmitSEHTryStmt(const SEHTryStmt &S);
>> - void EmitCXXForRangeStmt(const CXXForRangeStmt &S);
>> + void EmitCXXForRangeStmt(const CXXForRangeStmt &S,
>> + ArrayRef<const Attr*> Attrs = ArrayRef<const Attr*>());
>
> Same here.
>
>>
>> llvm::Function *EmitCapturedStmt(const CapturedStmt &S, CapturedRegionKind K);
>> llvm::Function *GenerateCapturedStmtFunction(const CapturedDecl *CD,
>> Index: lib/Parse/ParsePragma.cpp
>> ===================================================================
>> --- lib/Parse/ParsePragma.cpp (revision 208442)
>> +++ lib/Parse/ParsePragma.cpp (working copy)
>> @@ -15,6 +15,7 @@
>> #include "clang/Lex/Preprocessor.h"
>> #include "clang/Parse/ParseDiagnostic.h"
>> #include "clang/Parse/Parser.h"
>> +#include "clang/Sema/LoopHint.h"
>> #include "clang/Sema/Scope.h"
>> #include "llvm/ADT/StringSwitch.h"
>> using namespace clang;
>> @@ -131,6 +132,12 @@
>> Token &FirstToken) override;
>> };
>>
>> +struct PragmaLoopHintHandler : public PragmaHandler {
>> + PragmaLoopHintHandler() : PragmaHandler("loop") {}
>> + void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer,
>> + Token &FirstToken) override;
>> +};
>> +
>> } // end namespace
>>
>> void Parser::initializePragmaHandlers() {
>> @@ -195,6 +202,9 @@
>> MSSection.reset(new PragmaMSPragma("section"));
>> PP.AddPragmaHandler(MSSection.get());
>> }
>> +
>> + LoopHintHandler.reset(new PragmaLoopHintHandler());
>> + PP.AddPragmaHandler(LoopHintHandler.get());
>> }
>>
>> void Parser::resetPragmaHandlers() {
>> @@ -249,6 +259,10 @@
>>
>> PP.RemovePragmaHandler("STDC", FPContractHandler.get());
>> FPContractHandler.reset();
>> +
>> + PP.RemovePragmaHandler(LoopHintHandler.get());
>> + LoopHintHandler.reset();
>> +
>> }
>>
>> /// \brief Handle the annotation token produced for #pragma unused(...)
>> @@ -570,6 +584,40 @@
>> DiagnosticsEngine::Error, "'#pragma %0' not implemented.");
>> }
>>
>> +struct PragmaLoopHintInfo {
>> + Token Loop;
>> + Token Value;
>> + Token Option;
>> +};
>> +
>> +LoopHint Parser::HandlePragmaLoopHint() {
>> + assert(Tok.is(tok::annot_pragma_loop_hint));
>> + PragmaLoopHintInfo *Info =
>> + static_cast<PragmaLoopHintInfo *>(Tok.getAnnotationValue());
>> +
>> + LoopHint Hint;
>> + Hint.LoopLoc = IdentifierLoc::create(Actions.Context,
>> + Info->Loop.getLocation(),
>> + Info->Loop.getIdentifierInfo());
>> + Hint.OptionLoc = IdentifierLoc::create(Actions.Context,
>> + Info->Option.getLocation(),
>> + Info->Option.getIdentifierInfo());
>> + Hint.ValueLoc = IdentifierLoc::create(Actions.Context,
>> + Info->Value.getLocation(),
>> + Info->Value.getIdentifierInfo());
>
> Identation on all of these.
>
>> + Hint.Range = SourceRange(Info->Option.getLocation(),
>> + Info->Value.getLocation());
>> +
>> + // FIXME: We should support template parameters for the loop hint value.
>> + // See bug report #19610
>> + if (Info->Value.is(tok::numeric_constant))
>> + Hint.ValueExpr = Actions.ActOnNumericConstant(Info->Value).get();
>> + else
>> + Hint.ValueExpr = nullptr;
>> +
>> + return Hint;
>> +}
>> +
>> // #pragma GCC visibility comes in two variants:
>> // 'push' '(' [visibility] ')'
>> // 'pop'
>> @@ -1531,3 +1579,112 @@
>>
>> Actions.ActOnPragmaMSComment(Kind, ArgumentString);
>> }
>> +
>> +/// \brief Handle the \#pragma vectorize hint.
>> +/// #pragma 'loop' loop-hints
>> +///
>> +/// loop-hints:
>> +/// loop-hint loop-hints[opt]
>> +///
>> +/// loop-hint:
>> +/// 'vectorize' '(' loop-hint-value ')'
>> +/// 'interleave' '(' loop-hint-value ')'
>> +///
>> +/// loop-hint-value:
>> +/// 'enable'
>> +/// 'disable'
>> +/// constant-expression
>> +///
>> +/// Specifying vectorize(enable) or vectorize(_value_) instructs llvm to
>> +/// try vectorize the instructions of the loop it precedes. Specifying
>
> "to try vectorize" doesn't sound right.
>
>> +/// interleave(enable) or interleave(_value_) instructs llvm to interleaving
>> +/// multiple iterations of the loop it precedes. The _value_ indicates the
>
> "to interleaving multiple iterations" doesn't sound right either.
>
>> +/// width of the vector instructions or the number of iterations of the loop
>> +/// that should be interleaved. Consequently, a value of 1 or disable prevents
>> +/// the optimization, even if it is possible and profitable, and 0 is invalid.
>> +/// The loop vectorizer currently only works on inner loops.
>> +///
>> +void PragmaLoopHintHandler::HandlePragma(Preprocessor &PP,
>> + PragmaIntroducerKind Introducer,
>> + Token &Tok) {
>> + Token Loop = Tok;
>> + SmallVector <Token, 1> TokenList;
>> +
>> + // Lex the optimization option and verify it is an identifier.
>> + PP.Lex(Tok);
>> + if (Tok.isNot(tok::identifier)) {
>> + bool InvalidOption = true;
>
> Why is this variable needed?
>
>> + PP.Diag(Tok.getLocation(), diag::err_pragma_loop_invalid_option)
>> + << InvalidOption << "";
>> + return;
>> + }
>> +
>> + while(Tok.is(tok::identifier)) {
>> + Token Option = Tok;
>> + StringRef OptionName = Tok.getIdentifierInfo()->getName();
>> +
>> + if (OptionName != "vectorize" && OptionName != "interleave") {
>> + bool InvalidOption = false;
>
> Same for this variable.
>
> In both cases, the idiomatic approach is to use 0/1/true/false and an
> inline comment.
>
>> + PP.Diag(Tok.getLocation(), diag::err_pragma_loop_invalid_option)
>> + << InvalidOption << OptionName;
>> + return;
>> + }
>> +
>> + // Read '('
>> + PP.Lex(Tok);
>> + if (Tok.isNot(tok::l_paren)) {
>
> Hmmm, maybe someday we should generalize BalancedDelimiterTracker so
> it would work in the preprocessor as well. Oh well, that's a task for
> another day.
>
>> + PP.Diag(Tok.getLocation(), diag::err_expected) << tok::l_paren;
>> + return;
>> + }
>> +
>> + // Read the optimization value identifier.
>
> Comment doesn't match the code.
>
>> + PP.Lex(Tok);
>> + if (Tok.isNot(tok::identifier) && Tok.isNot(tok::numeric_constant)) {
>> + PP.Diag(Tok.getLocation(), diag::err_pragma_loop_invalid_type)
>> + << Tok.getName();
>> + return;
>> + }
>> +
>> + Token Value = Tok;
>> +
>> + // Read ')'
>> + PP.Lex(Tok);
>> + if (Tok.isNot(tok::r_paren)) {
>> + PP.Diag(Tok.getLocation(), diag::err_expected) << tok::r_paren;
>> + return;
>> + }
>> +
>> + // Get next optimization option.
>> + PP.Lex(Tok);
>> +
>> + // Read the optimization option identifier.
>
> Comment doesn't match code.
>
>> + PragmaLoopHintInfo *Info =
>> + (PragmaLoopHintInfo*) PP.getPreprocessorAllocator().Allocate(
>> + sizeof(PragmaLoopHintInfo), llvm::alignOf<PragmaLoopHintInfo>());
>> +
>> + Info->Loop = Loop;
>> + Info->Option = Option;
>> + Info->Value = Value;
>> +
>> + // Generate the vectorization hint token.
>> + Token LoopHintTok;
>> + LoopHintTok.startToken();
>> + LoopHintTok.setKind(tok::annot_pragma_loop_hint);
>> + LoopHintTok.setLocation(Loop.getLocation());
>> + LoopHintTok.setAnnotationValue(static_cast<void*>(Info));
>> + TokenList.push_back(LoopHintTok);
>> + }
>> +
>> + if (Tok.isNot(tok::eod)) {
>> + PP.Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
>> + << "loop";
>> + return;
>> + }
>> +
>> + Token *TokenArray = new Token[TokenList.size()];
>> + std::copy(TokenList.begin(), TokenList.end(), TokenArray);
>> +
>> + PP.EnterTokenStream(TokenArray, TokenList.size(),
>> + /*DisableMacroExpansion=*/false,
>> + /*OwnsTokens=*/true);
>> +}
>> Index: lib/Parse/ParseStmt.cpp
>> ===================================================================
>> --- lib/Parse/ParseStmt.cpp (revision 208442)
>> +++ lib/Parse/ParseStmt.cpp (working copy)
>> @@ -15,11 +15,13 @@
>> #include "clang/Parse/Parser.h"
>> #include "RAIIObjectsForParser.h"
>> #include "clang/AST/ASTContext.h"
>> +#include "clang/Basic/Attributes.h"
>> #include "clang/Basic/Diagnostic.h"
>> #include "clang/Basic/PrettyStackTrace.h"
>> #include "clang/Basic/SourceManager.h"
>> #include "clang/Basic/TargetInfo.h"
>> #include "clang/Sema/DeclSpec.h"
>> +#include "clang/Sema/LoopHint.h"
>> #include "clang/Sema/PrettyDeclStackTrace.h"
>> #include "clang/Sema/Scope.h"
>> #include "clang/Sema/TypoCorrection.h"
>> @@ -357,6 +359,10 @@
>> ProhibitAttributes(Attrs);
>> HandlePragmaMSPragma();
>> return StmtEmpty();
>> +
>> + case tok::annot_pragma_loop_hint:
>> + ProhibitAttributes(Attrs);
>> + return ParsePragmaLoopHint(Stmts, OnlyStatement, TrailingElseLoc, Attrs);
>> }
>>
>> // If we reached this code, the statement must end in a semicolon.
>> @@ -1751,6 +1757,43 @@
>> return Actions.ActOnReturnStmt(ReturnLoc, R.take(), getCurScope());
>> }
>>
>> +StmtResult Parser::ParsePragmaLoopHint(StmtVector &Stmts,
>> + bool OnlyStatement,
>> + SourceLocation *TrailingElseLoc,
>> + ParsedAttributesWithRange &Attrs)
>> +{
>
> Curly brace should follow the previous line.
>
>> + // Create temporary attribute list.
>> + ParsedAttributesWithRange TempAttrs(AttrFactory);
>> +
>> + // Get vectorize hints and consume annotated token.
>> + while (Tok.is(tok::annot_pragma_loop_hint)) {
>> + LoopHint Hint = HandlePragmaLoopHint();
>> + ConsumeToken();
>> +
>> + if (!Hint.LoopLoc ||
>> + !Hint.OptionLoc ||
>> + !Hint.ValueLoc)
>> + continue;
>> +
>> + ArgsUnion ArgHints[] = {Hint.OptionLoc,
>> + Hint.ValueLoc,
>> + ArgsUnion(Hint.ValueExpr)};
>> + TempAttrs.addNew(Hint.LoopLoc->Ident, Hint.Range,
>> + 0, Hint.LoopLoc->Loc,
>> + ArgHints, 3,
>> + AttributeList::AS_Keyword);
>
> This should have a FIXME for when we do AttributeList::AS_Pragma.
>
>> + }
>> +
>> + // Get the next statement.
>> + MaybeParseCXX11Attributes(Attrs);
>> +
>> + StmtResult S = ParseStatementOrDeclarationAfterAttributes(Stmts,
>> + /*OnlyStatement*/ true, 0, Attrs);
>
> Shouldn't we be passing the OnlyStatement which was passed into the
> function? Same for passing in the TrailingElseLoc instead of 0?
>
>> +
>> + Attrs.takeAllFrom(TempAttrs);
>> + return S;
>> +}
>> +
>> namespace {
>> class ClangAsmParserCallback : public llvm::MCAsmParserSemaCallback {
>> Parser &TheParser;
>> Index: lib/Sema/SemaStmtAttr.cpp
>> ===================================================================
>> --- lib/Sema/SemaStmtAttr.cpp (revision 208442)
>> +++ lib/Sema/SemaStmtAttr.cpp (working copy)
>> @@ -14,8 +14,10 @@
>> #include "clang/Sema/SemaInternal.h"
>> #include "clang/AST/ASTContext.h"
>> #include "clang/Basic/SourceManager.h"
>> +#include "clang/Lex/Lexer.h"
>
> This isn't required to be included; please remove.
>
>> #include "clang/Sema/DelayedDiagnostic.h"
>> #include "clang/Sema/Lookup.h"
>> +#include "clang/Sema/LoopHint.h"
>> #include "clang/Sema/ScopeInfo.h"
>> #include "llvm/ADT/StringExtras.h"
>>
>> @@ -42,9 +44,82 @@
>> A.getAttributeSpellingListIndex());
>> }
>>
>> +struct PreviousKind {
>> + int Vectorize;
>> + int Interleave;
>> +};
>>
>> +static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const AttributeList &A,
>> + SourceRange Range, PreviousKind &Prev) {
>> + if (St->getStmtClass() != Stmt::DoStmtClass &&
>> + St->getStmtClass() != Stmt::ForStmtClass &&
>> + St->getStmtClass() != Stmt::CXXForRangeStmtClass &&
>> + St->getStmtClass() != Stmt::WhileStmtClass) {
>> + S.Diag(St->getLocStart(), diag::err_pragma_loop_precedes_nonloop);
>> + return 0;
>> + }
>> +
>> + IdentifierLoc *OptionLoc = A.getArgAsIdent(0);
>> + IdentifierInfo *OptionInfo = OptionLoc->Ident;
>> + IdentifierLoc *ValueLoc = A.getArgAsIdent(1);
>> + IdentifierInfo *ValueInfo = ValueLoc->Ident;
>> + Expr *ValueExpr = A.getArgAsExpr(2);
>> +
>> + LoopHintAttr::OptionType Option = LoopHintAttr::Vectorize;
>> + if (OptionInfo && OptionInfo->getName() == "vectorize")
>> + Option = LoopHintAttr::Vectorize;
>> + else if (OptionInfo && OptionInfo->getName() == "interleave")
>> + Option = LoopHintAttr::Interleave;
>
> You should be able to simply assert OptionInfo, since the parser won't
> generate an annotation token if the pragma was ill-formed.
>
>> +
>> + LoopHintAttr::KindType Kind = LoopHintAttr::Value;
>> + if (ValueInfo && ValueInfo->getName() == "enable")
>> + Kind = LoopHintAttr::Enable;
>> + else if (ValueInfo && ValueInfo->getName() == "disable")
>> + Kind = LoopHintAttr::Disable;
>> +
>> + // We only need to check that a loop hint is compatible with the
>> + // previous loop hint to ensure that all hints are compatible.
>> + if (Option == LoopHintAttr::Vectorize) {
>> + if (Prev.Vectorize != -1 &&
>> + !LoopHintAttr::isCompatible(Prev.Vectorize, Kind)) {
>> + S.Diag(ValueLoc->Loc, diag::err_pragma_loop_incompatible)
>> + << LoopHintAttr::getKindName(Kind)
>> + << LoopHintAttr::getKindName(Prev.Vectorize)
>> + << LoopHintAttr::getOptionName(Option);
>> + return 0;
>> + }
>> + Prev.Vectorize = Kind;
>> + } else if (Option == LoopHintAttr::Interleave) {
>> + if (Prev.Interleave != -1 &&
>> + !LoopHintAttr::isCompatible(Prev.Interleave, Kind)) {
>> + S.Diag(ValueLoc->Loc, diag::err_pragma_loop_incompatible)
>> + << LoopHintAttr::getKindName(Kind)
>> + << LoopHintAttr::getKindName(Prev.Interleave)
>> + << LoopHintAttr::getOptionName(Option);
>> + return 0;
>> + }
>> + Prev.Interleave = Kind;
>> + }
>
> It seems like this could be simplified to remove all of the duplicate
> code. Prev doesn't need Interleave and Vectorize to be separate
> members since only one is active at a time.
>
>> +
>> + // FIXME: We should support template parameters for the loop hint value.
>> + // See bug report #19610
>> + if (Kind == LoopHintAttr::Value) {
>> + llvm::APSInt ValueAPS;
>> + if(!ValueExpr ||
>> + !ValueExpr->isIntegerConstantExpr(ValueAPS, S.Context) ||
>> + (int)ValueAPS.getSExtValue() < 1) {
>> + S.Diag(ValueLoc->Loc, diag::err_pragma_loop_invalid_value)
>> + << LoopHintAttr::getOptionName(Option);
>> + return 0;
>> + }
>> + }
>> +
>> + return LoopHintAttr::CreateImplicit(S.Context,
>> + Option, Kind, ValueExpr, A.getRange());
>
> Formatting.
>
>> +}
>
> The "return 0" statements in this function should all be "return
> nullptr" instead.
>
>> +
>> static Attr *ProcessStmtAttribute(Sema &S, Stmt *St, const AttributeList &A,
>> - SourceRange Range) {
>> + SourceRange Range, PreviousKind &Prev) {
>
> No, this definitely cannot accept a PreviousKind object. The
> handleFooAttr functions all need to have a uniform parameter list
> unless there's an exceptional reason against it (I've not gotten
> around to harmonizing statement and type attributes the same way I
> have with the declaration ones, which is a pain point I wish to avoid
> in the future). All of those functions key off the signature for this
> one, so handleLoopHintAttr's signature needs to drop the PreviousKind
> param as well.
>
> You can't solve this the same way it's solved in SemaDeclAttr.cpp
> since you cannot simply ask the statement for its attributes. Instead,
> I think ProcessStmtAttributes should take a second pass over the Attrs
> vector to test for mutual exclusion, invalid duplicates, etc and allow
> for removing attributes as-needed from that list before calling
> ActOnAttributedStmt.
>
>> switch (A.getKind()) {
>> case AttributeList::UnknownAttribute:
>> S.Diag(A.getLoc(), A.isDeclspecAttribute() ?
>> @@ -53,6 +128,8 @@
>> return 0;
>> case AttributeList::AT_FallThrough:
>> return handleFallThroughAttr(S, St, A, Range);
>> + case AttributeList::AT_LoopHint:
>> + return handleLoopHintAttr(S, St, A, Range, Prev);
>> default:
>> // if we're here, then we parsed a known attribute, but didn't recognize
>> // it as a statement attribute => it is declaration attribute
>> @@ -64,9 +141,14 @@
>>
>> StmtResult Sema::ProcessStmtAttributes(Stmt *S, AttributeList *AttrList,
>> SourceRange Range) {
>> + PreviousKind Prev;
>> + Prev.Vectorize = -1;
>> + Prev.Interleave = -1;
>> +
>> SmallVector<const Attr*, 8> Attrs;
>> + //Note: Attrs are stored in the reverse order than they appear.
>> for (const AttributeList* l = AttrList; l; l = l->getNext()) {
>> - if (Attr *a = ProcessStmtAttribute(*this, S, *l, Range))
>> + if (Attr *a = ProcessStmtAttribute(*this, S, *l, Range, Prev))
>> Attrs.push_back(a);
>> }
>>
>> Index: test/CodeGen/pragma-loop.cpp
>> ===================================================================
>> --- test/CodeGen/pragma-loop.cpp (revision 0)
>> +++ test/CodeGen/pragma-loop.cpp (working copy)
>> @@ -0,0 +1,120 @@
>> +// RUN: %clang -std=c++11 -emit-llvm -S -o - %s | FileCheck %s
>> +
>> +// CHECK: br i1 %cmp, label %while.body, label %while.end, !llvm.loop !1
>> +// CHECK: br i1 %cmp, label %do.body, label %do.end, !llvm.loop !5
>> +// CHECK: br i1 %cmp, label %for.body, label %for.end, !llvm.loop !7
>> +// CHECK: br i1 %cmp, label %for.body, label %for.end, !llvm.loop !8
>> +// CHECK: br i1 %cmp, label %for.body, label %for.end, !llvm.loop !11
>> +// CHECK: br i1 %cmp, label %for.body, label %for.end, !llvm.loop !13
>> +// CHECK: br i1 %cmp, label %for.body, label %for.end, !llvm.loop !14
>> +// CHECK: br i1 %cmp, label %for.body, label %for.end, !llvm.loop !16
>> +
>> +// CHECK: !1 = metadata !{metadata !1, metadata !2, metadata !3, metadata !4}
>> +// CHECK: !2 = metadata !{metadata !"llvm.vectorizer.enable", i1 true}
>> +// CHECK: !3 = metadata !{metadata !"llvm.vectorizer.unroll", i32 4}
>> +// CHECK: !4 = metadata !{metadata !"llvm.vectorizer.width", i32 4}
>> +// CHECK: !5 = metadata !{metadata !5, metadata !6, metadata !3}
>> +// CHECK: !6 = metadata !{metadata !"llvm.vectorizer.width", i32 8}
>> +// CHECK: !7 = metadata !{metadata !7, metadata !2, metadata !3}
>> +// CHECK: !8 = metadata !{metadata !8, metadata !9, metadata !10}
>> +// CHECK: !9 = metadata !{metadata !"llvm.vectorizer.width", i32 2}
>> +// CHECK: !10 = metadata !{metadata !"llvm.vectorizer.unroll", i32 2}
>> +// CHECK: !11 = metadata !{metadata !11, metadata !12}
>> +// CHECK: !12 = metadata !{metadata !"llvm.vectorizer.width", i32 1}
>> +// CHECK: !13 = metadata !{metadata !13, metadata !9, metadata !10}
>> +// CHECK: !14 = metadata !{metadata !14, metadata !6, metadata !15}
>> +// CHECK: !15 = metadata !{metadata !"llvm.vectorizer.unroll", i32 8}
>> +// CHECK: !16 = metadata !{metadata !16, metadata !9, metadata !10}
>> +
>> +// Verify while loop is recognized after sequence of pragma loop directives.
>> +void while_test(int *List, int Length) {
>> + int i = 0;
>> +
>> + #pragma loop vectorize(enable)
>> + #pragma loop interleave(4)
>> + #pragma loop vectorize(4)
>> + while(i < Length) {
>> + List[i] = i*2;
>> + i++;
>> + }
>> +}
>> +
>> +// Verify do loop is recognized after multi-option pragma loop directive.
>> +void do_test(int *List, int Length) {
>> + int i = 0;
>> +
>> + #pragma loop vectorize(8) interleave(4)
>> + do {
>> + List[i] = i*2;
>> + i++;
>> + } while (i < Length);
>> +}
>> +
>> +// Verify for loop is recognized after sequence of pragma loop directives.
>> +void for_test(int *List, int Length) {
>> + #pragma loop interleave(enable)
>> + #pragma loop interleave(4)
>> + for(int i = 0; i < Length; i++) {
>> + List[i] = i*2;
>> + }
>> +}
>> +
>> +// Verify c++11 for range loop is recognized after
>> +// sequence of pragma loop directives.
>> +void for_range_test() {
>> + double List[100];
>> +
>> + #pragma loop vectorize(2) interleave(2)
>> + for (int i : List) {
>> + List[i] = i;
>> + }
>> +}
>> +
>> +// Verify disable pragma loop directive generates correct metadata
>> +void disable_test(int *List, int Length) {
>> + #pragma loop vectorize(disable)
>> + for(int i = 0; i < Length; i++) {
>> + List[i] = i*2;
>> + }
>> +}
>> +
>> +#define VECWIDTH 2
>> +#define INTERLEAVE 2
>> +
>> +// Verify defines are correctly resolved in pragma loop directive
>> +void for_define_test(int *List, int Length, int Value) {
>> + #pragma loop vectorize(VECWIDTH) interleave(INTERLEAVE)
>> + for(int i = 0; i < Length; i++) {
>> + List[i] = i*Value;
>> + }
>> +}
>> +
>> +// Verify metadata is generated when template is used.
>> +template <typename A>
>> +void for_template_test(A *List, int Length, A Value) {
>> +
>> + #pragma loop vectorize(8) interleave(8)
>> + for(int i = 0; i < Length; i++) {
>> + List[i] = i*Value;
>> + }
>> +}
>> +
>> +// Verify define is resolved correctly when template is used.
>> +template <typename A>
>> +void for_template_define_test(A *List, int Length, A Value) {
>> + #pragma loop vectorize(VECWIDTH) interleave(INTERLEAVE)
>> + for(int i = 0; i < Length; i++) {
>> + List[i] = i*Value;
>> + }
>> +}
>> +
>> +#undef VECWIDTH
>> +#undef INTERLEAVE
>> +
>> +// Use templates defined above. Test verifies metadata is generated correctly.
>> +void template_test(double *List, int Length) {
>> + double Value = 10;
>> +
>> + for_template_test<double>(List, Length, Value);
>> + for_template_define_test<double>(List, Length, Value);
>> +}
>> Index: test/PCH/pragma-loop.cpp
>> ===================================================================
>> --- test/PCH/pragma-loop.cpp (revision 0)
>> +++ test/PCH/pragma-loop.cpp (working copy)
>> @@ -0,0 +1,59 @@
>> +// RUN: %clang_cc1 -emit-pch -o %t.a %s
>> +// RUN: %clang_cc1 -include-pch %t.a %s -ast-print -o - | FileCheck %s
>> +
>> +// CHECK: #pragma loop vectorize(8)
>> +// CHECK: #pragma loop interleave(4)
>> +// CHECK: #pragma loop vectorize(disable)
>> +// CHECK: #pragma loop interleave(enable)
>> +// CHECK: #pragma loop vectorize(enable)
>> +// CHECK: #pragma loop interleave(disable)
>> +
>> +#ifndef HEADER
>> +#define HEADER
>> +
>> +class pragma_test {
>> +public:
>> + inline void run1(int *List, int Length) {
>> + int i = 0;
>> + #pragma loop vectorize(8)
>> + #pragma loop interleave(4)
>> + while (i < Length) {
>> + List[i] = i;
>> + i++;
>> + }
>> + }
>> +
>> + inline void run2(int *List, int Length) {
>> + int i = 0;
>> + #pragma loop vectorize(disable)
>> + #pragma loop interleave(enable)
>> + while (i-1 < Length) {
>> + List[i] = i;
>> + i++;
>> + }
>> + }
>> +
>> + inline void run3(int *List, int Length) {
>> + int i = 0;
>> + #pragma loop vectorize(enable)
>> + #pragma loop interleave(disable)
>> + while (i-3 < Length) {
>> + List[i] = i;
>> + i++;
>> + }
>> + }
>> +};
>> +
>> +#else
>> +
>> +void test() {
>> + int List[100];
>> +
>> + pragma_test pt;
>> +
>> + pt.run1(List, 100);
>> + pt.run2(List, 100);
>> + pt.run3(List, 100);
>> +}
>> +
>> +#endif
>> Index: test/Parser/pragma-loop-ast.cpp
>> ===================================================================
>> --- test/Parser/pragma-loop-ast.cpp (revision 0)
>> +++ test/Parser/pragma-loop-ast.cpp (working copy)
>> @@ -0,0 +1,33 @@
>> +// RUN: %clang_cc1 -ast-print %s | FileCheck %s
>> +
>> +// CHECK: #pragma loop vectorize(4)
>> +// CHECK: #pragma loop interleave(8)
>> +// CHECK: #pragma loop vectorize(enable)
>> +// CHECK: #pragma loop interleave(disable)
>> +// CHECK: #pragma loop vectorize(disable)
>> +// CHECK: #pragma loop interleave(enable)
>> +
>> +void test(int *List, int Length)
>> +{
>> + int i = 0;
>> + #pragma loop vectorize(4)
>> + #pragma loop interleave(8)
>> + while (i < Length) {
>> + List[i] = i * 2;
>> + i++;
>> + }
>> +
>> + #pragma loop vectorize(enable)
>> + #pragma loop interleave(disable)
>> + while (i-1 < Length) {
>> + List[i] = i * 2;
>> + i++;
>> + }
>> +
>> + #pragma loop vectorize(disable)
>> + #pragma loop interleave(enable)
>> + while (i-2 < Length) {
>> + List[i] = i * 2;
>> + i++;
>> + }
>> +}
>> Index: test/Parser/pragma-loop.cpp
>> ===================================================================
>> --- test/Parser/pragma-loop.cpp (revision 0)
>> +++ test/Parser/pragma-loop.cpp (working copy)
>> @@ -0,0 +1,105 @@
>> +// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
>> +
>> +// Note that this puts the expected lines before the directives to work around
>> +// limitations in the -verify mode.
>> +
>> +void test(int *List, int Length)
>> +{
>> + int i = 0;
>> +
>> + #pragma loop vectorize(4)
>> + #pragma loop interleave(8)
>> + while (i+1 < Length) {
>> + List[i] = i;
>> + }
>> +
>> + #pragma loop vectorize(enable)
>> + #pragma loop interleave(enable)
>> + while (i < Length) {
>> + List[i] = i;
>> + }
>> +
>> + #pragma loop vectorize(disable)
>> + #pragma loop interleave(disable)
>> + while (i-1 < Length) {
>> + List[i] = i;
>> + }
>> +
>> + #pragma loop vectorize(4) interleave(8)
>> + while (i-2 < Length) {
>> + List[i] = i;
>> + }
>> +
>> + #pragma loop interleave(16)
>> + while (i-3 < Length) {
>> + List[i] = i;
>> + }
>> +
>> + int VList[Length];
>> + #pragma loop vectorize(disable) interleave(disable)
>> + for (int j : VList) {
>> + VList[j] = List[j];
>> + }
>> +
>> +/* expected-error {{expected '('}} */ #pragma loop vectorize
>> +/* expected-error {{expected '('}} */ #pragma loop interleave
>> +
>> +/* expected-error {{expected ')'}} */ #pragma loop vectorize(4
>> +/* expected-error {{expected ')'}} */ #pragma loop interleave(4
>> +
>> +/* expected-error {{missing option in directive '#pragma loop'}} */ #pragma loop
>> +/* expected-error {{invalid option 'badkeyword' in directive '#pragma loop'}} */ #pragma loop badkeyword
>> +/* expected-error {{invalid option 'badkeyword' in directive '#pragma loop'}} */ #pragma loop badkeyword(2)
>> +/* expected-error {{invalid option 'badkeyword' in directive '#pragma loop'}} */ #pragma loop vectorize(4) badkeyword(4)
>> +/* expected-warning {{extra tokens at end of '#pragma loop'}} */ #pragma loop vectorize(4) ,
>> +
>> + while (i-4 < Length) {
>> + List[i] = i;
>> + }
>> +
>> +/* expected-error {{expected a positive integer in directive '#pragma loop vectorize'}} */ #pragma loop vectorize(0)
>> +/* expected-error {{expected a positive integer in directive '#pragma loop interleave'}} */ #pragma loop interleave(0)
>> + while (i-5 < Length) {
>> + List[i] = i;
>> + }
>> +
>> +/* expected-error {{expected a positive integer in directive '#pragma loop vectorize'}} */ #pragma loop vectorize(3000000000)
>> +/* expected-error {{expected a positive integer in directive '#pragma loop interleave'}} */ #pragma loop interleave(3000000000)
>> + while (i-6 < Length) {
>> + List[i] = i;
>> + }
>> +
>> +/* expected-error {{expected a positive integer in directive '#pragma loop vectorize'}} */ #pragma loop vectorize(badvalue)
>> +/* expected-error {{expected a positive integer in directive '#pragma loop interleave'}} */ #pragma loop interleave(badvalue)
>> + while (i-7 < Length) {
>> + List[i] = i;
>> + }
>> +
>> +#pragma loop vectorize(enable)
>> +/* expected-error {{expected a for, while, or do-while loop to follow the '#pragma loop' directive}} */ int j = Length;
>> + List[0] = List[1];
>> +
>> + while (j-1 < Length) {
>> + List[j] = j;
>> + }
>> +
>> +/* expected-error {{'value' and 'disable' directive option types are incompatible in '#pragma loop vectorize'}} */ #pragma loop vectorize(4)
>> + #pragma loop vectorize(disable)
>> + while (i-8 < Length) {
>> + List[i] = i;
>> + }
>> +
>> +/* expected-error {{'enable' and 'disable' directive option types are incompatible in '#pragma loop interleave'}} */ #pragma loop interleave(enable)
>> + #pragma loop interleave(disable)
>> + while (i-9 < Length) {
>> + List[i] = i;
>> + }
>> +
>> +/* expected-error {{'disable' and 'value' directive option types are incompatible in '#pragma loop vectorize'}} */ #pragma loop vectorize(disable)
>> + #pragma loop vectorize(4)
>> + while (i-10 < Length) {
>> + List[i] = i;
>> + }
>> +
>> +#pragma loop interleave(enable)
>> +/* expected-error {{expected statement}} */ }
>>
>
> Btw, when I test your patch locally, I get failed assertions from the
> STL. "array iterator + offset out of range" on a call to std::copy
> within ASTStmtReader::VisitAttributedStmt().
>
> ~Aaron
More information about the cfe-commits
mailing list