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

[Alexey Bataev]

John, thank you for the review!


================
Comment at: include/clang/AST/ASTContext.h:89
@@ -85,2 +88,3 @@
+          AlignIsRequired(AlignIsRequired) {}
   };
 
----------------
rjmccall wrote:
> Just make this a separate query on ASTContext.  TypeInfo computation is important to a lot of different clients, basically none of which care about this.
> 
> I don't think it's important to cache.
Ok, will separate it.

================
Comment at: include/clang/Basic/TokenKinds.def:507
@@ +506,3 @@
+// OpenMP Type Traits
+KEYWORD(simd_align                  , KEYALL)
+
----------------
rjmccall wrote:
> This is a very general name to give this, and putting it in the unreserved namespace is problematic.  I think that, given some time and coordination, we could design a more general "what's the largest supported vector type for this underlying type" query, and that would justify a name like this.  But the existing vector attributes always talk about, well, "vector" instead of "simd"; and anyway, it's not obvious that the answer to that general query would necessarily dictate the answer to this OpenMP-specific one.
> 
> So let's design this as a more targeted feature around the needs of OpenMP.  In that context, "simd" is fine because it's exactly the name of the OpenMP feature.  How about "__builtin_openmp_simd_align"?  Or even "__builtin_openmp_required_simd_align"?
Ok, turned it to __builtin_omp_required_simd_align. Actually, I thought about such kind of name, but it was too long, so I changed it to a shorter version. :)

================
Comment at: lib/AST/ItaniumMangle.cpp:3031
@@ -3023,3 +3030,3 @@
       unsigned DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
                                      "cannot yet mangle vec_step expression");
       Diags.Report(DiagID);
----------------
rjmccall wrote:
> Copy-paste error.
Fixed, thanks

================
Comment at: lib/Basic/Targets.cpp:2977
@@ -2975,1 +2976,3 @@
+
+  SimdDefaultAlign = (getABI() == "avx") ? 32 : 16;
   return true;
----------------
rjmccall wrote:
> Aren't we saying that this is 64 for AVX512?
There were no "avx512" defined when I prepared a patch. Will be added

================
Comment at: lib/CodeGen/CGExprScalar.cpp:2044
@@ -2040,1 +2043,3 @@
+        CGF.getContext().getTypeInfo(E->getTypeOfArgument()).SimdDefaultAlign;
+    return Builder.getInt32(Alignment);
   }
----------------
rjmccall wrote:
> This needs to produce a size_t, not an int32_t.  You can use CGM::getSize.
Thanks, fixed

================
Comment at: lib/Sema/SemaExpr.cpp:3522
@@ -3522,1 +3521,3 @@
+      (TraitKind == UETT_SizeOf || TraitKind == UETT_AlignOf ||
+       TraitKind == UETT_OpenMPDefaultSimdAlign)) {
     // sizeof(function)/alignof(function) is allowed as an extension.
----------------
rjmccall wrote:
> This should be a hard error, not just an extension warning.
Will be removed it and expressions won't be allowed as arguments

================
Comment at: lib/Sema/SemaExpr.cpp:3831
@@ -3827,1 +3830,3 @@
+    isInvalid =
+        CheckUnaryExprOrTypeTraitOperand(E, UETT_OpenMPDefaultSimdAlign);
   } else if (E->refersToBitField()) {  // C99 6.5.3.4p1.
----------------
rjmccall wrote:
> Hmm.  Thinking more about it, using this trait with an expression instead of a type seems pretty misleading.  Let's make this a hard error for now.  We can continue to use the UETT parsing logic and AST representation.
Ok

http://reviews.llvm.org/D10597

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