On Wed, May 14, 2014 at 12:51 AM, Alexey Bataev <[email protected]> wrote:
> I think CodeGen part of the patch should be reviewed by  John McCall.
>
> Some small comments.
>
> @@ -502,7 +519,57 @@
>
>    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);
> +  for (auto I = Attrs.begin(), E = Attrs.end(); I != E; ++I) {
>
> Maybe it would be better to rewrite for the following way "for (auto Attr :
> Attrs)"?
>
> ===================================================================
> --- lib/Sema/SemaStmtAttr.cpp    (revision 208719)
> +++ lib/Sema/SemaStmtAttr.cpp    (working copy)
>
> @@ -16,6 +16,7 @@
>  #include "clang/Basic/SourceManager.h"
>  #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,7 +43,81 @@
>
> A.getAttributeSpellingListIndex());
>  }
>
> +static Attr *handleLoopHintAttr(Sema &S, Stmt *St, const AttributeList &A,
> +                                SourceRange Range) {
>
> Argument "Range" is not used.

Please leave the type in the function's signature (so it remains
consistent with other handleFooAttr functions in the file), but it's
okay to remove the identifier for the type.

~Aaron
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to