[PATCH] [OPENMP] Introduced type trait "simd_align" for default simd alignment.

John McCall rjmccall at gmail.com
Thu Jun 25 15:43:55 PDT 2015


"omp" instead of "openmp" makes a lot sense, since that's what's used in the pragma.


================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:4688
@@ -4688,2 +4687,3 @@
+  "invalid application of '%select{sizeof|alignof|vec_step|__builtin_omp_required_simd_align} to a "
   "function type">, InGroup<PointerArith>;
 def ext_sizeof_alignof_void_type : Extension<
----------------
You lost the 0 here.

================
Comment at: include/clang/Basic/TargetInfo.h:397
@@ -395,1 +396,3 @@
   unsigned getMaxVectorAlign() const { return MaxVectorAlign; }
+  /// \brief Return default simd alignment for the given target.
+  unsigned getSimdDefaultAlign() const { return SimdDefaultAlign; }
----------------
Please embellish this comment to make it clear that it's sometimes type-specific.

It would be great if we could move the type-specific logic to TargetInfo, but I don't know of a way to do that without either (1) making a lot of assumptions about what kinds of types are interesting to the target or (2) creating a cyclic dependency between lib/Basic and lib/AST.  Ugh.  We can stick with what we have right now.

================
Comment at: lib/AST/ASTContext.cpp:1791
@@ +1790,3 @@
+  unsigned SimdAlign = getTargetInfo().getSimdDefaultAlign();
+  // Target ppc64 with QPX: simd default alignment for pointer to double is 32.
+  if ((getTargetInfo().getTriple().getArch() == llvm::Triple::ppc64 ||
----------------
I think this query should probably apply to the underlying type, not to the pointer type.  That is, if I want to know the expected alignment of an array of doubles, I should use
  __builtin_omp_required_simd_align(double)
not
  __builtin_omp_required_simd_align(double*)

Unless PPC64's rule really is about arrays of pointers to double?

================
Comment at: lib/CodeGen/CGExprScalar.cpp:2045
@@ +2044,3 @@
+            .toCharUnitsFromBits(CGF.getContext().getOpenMPDefaultSimdAlign(
+                E->getTypeOfArgument()))
+            .getQuantity();
----------------
I assume the type of the argument here is always a pointer, which means you'll need to drill down to the element type.

================
Comment at: lib/Parse/ParseExpr.cpp:1046
@@ -1045,1 +1045,3 @@
+  // unary-expression: OpenMP 'default simd alignment' expression
+  case tok::kw___builtin_omp_required_simd_align:
     return ParseUnaryExprOrTypeTraitExpression();
----------------
These comments are supposed to be grammar specifications; whoever implemented vec_step apparently missed that.

What you want is:
  unary-expression: '__builtin_omp_required_simd_align' '(' type-name ')'
(since we don't actually allow the expression form semantically)

http://reviews.llvm.org/D10597

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list