[PATCH] #pragma vectorize

[Alexey Bataev]

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.


Best regards,
Alexey Bataev
=============
Software Engineer
Intel Compiler Team
Intel Corp.

13.05.2014 23:22, Tyler Nowicki пишет:
> Hi Aaron, Richard,
>
>> After committing r208702/3, your tests now pass for me. I'll take it
>> on faith that the comment and formatting gets resolved when you
>> commit, so LGTM. You should wait for Richard to sign off as well,
>> since he had some comments.
>
> Thanks for all the reviewing, I’m glad this is getting closer to being committed!
>
> @Richard, please take a look at the attached patch.
>
>
>> As it's written, it implies that you must specify vectorize or
>> interleave, followed by disable/enable/value, and then optionally
>> supply a value. Eg) #pragma loop vectorize(enable, 4)
>>
>> That's why I think we should have a Union argument type because you
>> really want it to either be enable|disable, or an integer value, not
>> both.
>>
>> Not something that needs doing for this patch by any means. :-)
> I’ll do that as a follow-up.
>
>
>>> I'd be nice to have a command-line switch to turn on and off this pragma.
>>> For example skipping "-fopenmp" makes a compiler to ignore OMP pragmas.
>>> So if you add this switch one can analyse the pure effect of using
>>> vectorizing pragmas at the specified locations.
>> Couldn't that be accomplished via macros already, where you have a
>> macro definition for #pragma loop, and simply noop the macro from the
>> command line with -D?
>
> LLVM doesn’t seem to allow you to define a pragma in a macro. It seems to expect the macro to only contain expressions. This is probably a bug?
>
>
>> When you reintroduce your changes, I think I would prefer to leave the
>> iteration in its current (forward) order instead of reverse order (as
>> you have it in your patch). That's simply a bug, and working around it
>> here is likely to ensure that bug sticks around even longer. Once you
>> have introduced your changes, I'd appreciate an extra test case which
>> uses -ast-print and demonstrates the reversed order (I am okay with us
>> XFAILing that test) so that gives us a target test to use to verify
>> the fix when it does happen.
> I’ll add the test case in a separate patch since it isn’t related to this work. I’m not really thrilled about the compiler producing pragmas in the reverse order in the mean time though. I updated the patch and tests so they pass with the forward iteration (reverse order) attributes.
>
> Tyler
>