Re: [PATCH] [OPENMP] Codegen of the ‘aligned’ clause for the ‘omp simd’ directive

[hfinkel at anl.gov]

================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:64
@@ +63,3 @@
+    Alignment = CGM.getTargetCodeGenInfo().getOpenMPSimdDefaultAlignment();
+    if (!llvm::isPowerOf2_32(Alignment))
+      Alignment = 0;
----------------
I think that you should just assert that this is either a power of 2 (or zero): if a target starts returning some other kind of number, that would be a bug, and we'd want to know.

================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:69
@@ +68,3 @@
+    assert(llvm::isPowerOf2_32(Alignment) && "alignment is not power of 2");
+    for (auto E : Clause.varlists()) {
+      llvm::Value *PtrValue = CGF.EmitScalarExpr(E);
----------------
As I also mention below, when we use a default alignment, I'll need that to depend on the type, so you'll need to rearrange things here slightly.

================
Comment at: lib/CodeGen/CGStmtOpenMP.cpp:94
@@ -68,1 +93,3 @@
     }
+    case OMPC_aligned: {
+      EmitOMPAlignedClause(*this, CGM, cast<OMPAlignedClause>(*C));
----------------
You don't need the {} here.

================
Comment at: lib/CodeGen/TargetInfo.h:223
@@ +222,3 @@
+  /// Gets the default (maximum) simd instructions alignment for target, for
+  /// the 'aligned' clause of the OpenMP directive 'simd'.
+  virtual unsigned getOpenMPSimdDefaultAlignment() const { return 0; }
----------------
I don't really like this description; how about this:

 /// Gets the target-specific default alignment used when an 'aligned' clause is used with a 'simd' OpenMP directive without specifying a specific alignment.


================
Comment at: lib/CodeGen/TargetInfo.h:224
@@ -221,1 +223,3 @@
+  /// the 'aligned' clause of the OpenMP directive 'simd'.
+  virtual unsigned getOpenMPSimdDefaultAlignment() const { return 0; }
 };
----------------
In general, I'll need this to depend on the type; please add a type parameter.

http://reviews.llvm.org/D5499