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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
