[PowerPC] Initial VSX intrinsic support, with min/max for vector double

[wschmidt]

Summary

Now that we have initial support for VSX, we can begin adding intrinsics for programmer access to VSX instructions. This patch performs the necessary enablement in the front end, and tests it by implementing intrinsics for minimum and maximum using the vector double data type.

The main change in the front end is to no longer disallow "vector" and "double" in the same declaration (lib/Sema/DeclSpec.cpp), but "vector" and "long double" must still be disallowed. The new intrinsics are accessed via vec_max and vec_min with changes in lib/Headers/altivec.h. Note that for v4f32, we already access corresponding VMX builtins, but with VSX enabled we should use the forms that allow all 64 vector registers.

The new builtins are defined in include/clang/Basic/BuiltinsPPC.def.

I've added a new test as test/CodeGen/builtins-ppc-vsx.c that is similar to, but much smaller than, builtins-ppc-altivec.c. This is intended to grow substantially over time, and allows us to test VSX IR generation without duplicating CHECK lines for the existing bazillion Altivec tests.

Since vector double is not legal, I've removed the expected error messages saying that it isn't from the existing tests test/Parser/altivec.c and test/Parser/cxx-altivec.cpp; and I've changed the expected error messages for 'vector long double'.

There is a companion patch for LLVM that will be reviewed separately; see http://reviews.llvm.org/D5948.

echristo: I added you as a reviewer, but please feel free to nominate a front-end personage to take your place. I'm not sure who should review the decl-spec parsing changes.

[hfinkel]

lib/Sema/DeclSpec.cpp

693

XL's docs say:

Note: The vector unsigned long long, vector bool long long, vector signed long long, and vector double types require architectures that support the VSX instruction set extensions, such as POWER7. You must specify the -qarch=pwr7 compiler option when you use these types.

I think it makes sense to do the corresponding thing in Clang. We should only make the VSX types available when PPCTargetInfo::getTargetDefines defines VSX, otherwise we should restrict to the Altivec types.

My motivation for saying this is that getting front-end syntax errors with nice error messages is preferable to getting 'cannot select' errors from the backend.

[wschmidt]

Hal, totally agree -- bad miss on my part. I'll fix that up.

[wschmidt]

This version now produces different error messages depending on whether or not VSX is enabled. If so, vector double is fine; if not, it's an error. I didn't have access to the TargetInfo information inside DeclSpec::SetTypeSpecType(), so I ended up having to pass that in from the caller. To minimize the amount of churn, I created a new overload of that function and only used the new version for the handling of double. I am certainly open to ideas of better ways to do this; this was the least invasive method I could think of.

[hfinkel]

I'd like to change the existing error message for use of double with __vector to also say something like, "; did you intend compile for the POWER7?"

Instead of adding a new overload, I think it would be better to just move the necessary type-checking logic into DeclSpec::Finish. DeclSpec::Finish already gets a Preprocessor &PP argument, and Preprocessor has a getTargetInfo function that you can use. The current scheme which depends on knowing to call the correct overload when adding DeclSpec::TST_double seems very fragile.

[wschmidt]

Just recording a discussion point from LLVMdev: Doing the check in DeclSpec::Finish() will work fine, but we need to be careful to get the caret for the diagnostic in the right place. This will require adding a ninth location that will track where the "double" showed up in the decl, and using that loc appropriately in the diagnostic calls.

[wschmidt]

Thanks for the suggestion, Hal -- this looks much cleaner to me. As a side benefit, I was able to clear up the weird behavior of giving both an error and a not-really-pertinent warning for "vector long double." There also wasn't an issue with the caret position, as I was able to just use the TST location in DeclSpec::Finish.

Let me know whether the wording of the new error message for vector double is ok.

[hfinkel]

LGTM, thanks!

include/clang/Basic/DiagnosticParseKinds.td

345

We're using the capitalization convention, right?

requires a target CPU of POWER7 or later

(might as well not skimp on the articles)

On the other hand, this could get a little misleading if compiling with -mcpu=power7 -mno-vsx, so maybe we should say:

requires VSX support to be enabled (available on the POWER7 or later)

[wschmidt]

We can go with POWER7. I came up with power7 because that is what we accept from -mcpu= on the command line. But I like your last suggestion, so let's go with that.

[wschmidt]

Thanks for the reviews and discussion! r220989.

include/clang/Basic/BuiltinsPPC.def

 
 BUILTIN(__builtin_altivec_vmaxsb, "V16ScV16ScV16Sc", "")
 BUILTIN(__builtin_altivec_vmaxub, "V16UcV16UcV16Uc", "")
 BUILTIN(__builtin_altivec_vmaxsh, "V8SsV8SsV8Ss", "")
 BUILTIN(__builtin_altivec_vmaxuh, "V8UsV8UsV8Us", "")
 BUILTIN(__builtin_altivec_vmaxsw, "V4SiV4SiV4Si", "")
 BUILTIN(__builtin_altivec_vmaxuw, "V4UiV4UiV4Ui", "")
 BUILTIN(__builtin_altivec_vmaxfp, "V4fV4fV4f", "")
+BUILTIN(__builtin_vsx_xvmaxdp,    "V2dV2dV2d", "")
+BUILTIN(__builtin_vsx_xvmaxsp,    "V4fV4fV4f", "")
+BUILTIN(__builtin_vsx_xsmaxdp,    "ddd", "")
 
 BUILTIN(__builtin_altivec_mfvscr, "V8Us", "")
 
 BUILTIN(__builtin_altivec_vminsb, "V16ScV16ScV16Sc", "")
 BUILTIN(__builtin_altivec_vminub, "V16UcV16UcV16Uc", "")
 BUILTIN(__builtin_altivec_vminsh, "V8SsV8SsV8Ss", "")
 BUILTIN(__builtin_altivec_vminuh, "V8UsV8UsV8Us", "")
 BUILTIN(__builtin_altivec_vminsw, "V4SiV4SiV4Si", "")
 BUILTIN(__builtin_altivec_vminuw, "V4UiV4UiV4Ui", "")
 BUILTIN(__builtin_altivec_vminfp, "V4fV4fV4f", "")
+BUILTIN(__builtin_vsx_xvmindp,    "V2dV2dV2d", "")
+BUILTIN(__builtin_vsx_xvminsp,    "V4fV4fV4f", "")
+BUILTIN(__builtin_vsx_xsmindp,    "ddd", "")
 
 BUILTIN(__builtin_altivec_mtvscr, "vV4i", "")
 
 BUILTIN(__builtin_altivec_vrefp, "V4fV4f", "")
 
 BUILTIN(__builtin_altivec_vrlb, "V16cV16cV16Uc", "")
 BUILTIN(__builtin_altivec_vrlh, "V8sV8sV8Us", "")
 BUILTIN(__builtin_altivec_vrlw, "V4iV4iV4Ui", "")

include/clang/Basic/DiagnosticParseKinds.td

def err_invalid_vector_decl_spec_combination : Error<
   "'__vector' must be first">;
 def err_invalid_pixel_decl_spec_combination : Error<
   "'__pixel' must be preceded by '__vector'.  "
   "'%0' declaration specifier not allowed here">;
 def err_invalid_vector_decl_spec : Error<
   "cannot use '%0' with '__vector'">;
 def err_invalid_vector_bool_decl_spec : Error<
   "cannot use '%0' with '__vector bool'">;
+def err_invalid_vector_long_double_decl_spec : Error<
+  "cannot use 'long double' with '__vector'">;
 def warn_vector_long_decl_spec_combination : Warning<
   "Use of 'long' with '__vector' is deprecated">, InGroup<Deprecated>;
 def err_friend_invalid_in_context : Error<
   "'friend' used outside of class">;
 def err_use_of_tag_name_without_tag : Error<
   "must use '%1' tag to refer to type %0%select{| in this scope}2">;
 def err_templated_using_directive : Error<
   "cannot template a using directive">;

include/clang/Sema/DeclSpec.h

 #ifndef LLVM_CLANG_SEMA_DECLSPEC_H
 #define LLVM_CLANG_SEMA_DECLSPEC_H
 
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/Basic/ExceptionSpecificationType.h"
 #include "clang/Basic/Lambda.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/Specifiers.h"
+#include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/Token.h"
 #include "clang/Sema/AttributeList.h"
 #include "clang/Sema/Ownership.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ErrorHandling.h"
 
 namespace clang {
bool SetTypeSpecWidth(TSW W, SourceLocation Loc, const char *&PrevSpec,
                         unsigned &DiagID, const PrintingPolicy &Policy);
   bool SetTypeSpecComplex(TSC C, SourceLocation Loc, const char *&PrevSpec,
                           unsigned &DiagID);
   bool SetTypeSpecSign(TSS S, SourceLocation Loc, const char *&PrevSpec,
                        unsigned &DiagID);
   bool SetTypeSpecType(TST T, SourceLocation Loc, const char *&PrevSpec,
                        unsigned &DiagID, const PrintingPolicy &Policy);
   bool SetTypeSpecType(TST T, SourceLocation Loc, const char *&PrevSpec,
+                       unsigned &DiagID, const PrintingPolicy &Policy,
+                       const TargetInfo& TI);
+  bool SetTypeSpecType(TST T, SourceLocation Loc, const char *&PrevSpec,
                        unsigned &DiagID, ParsedType Rep,
                        const PrintingPolicy &Policy);
   bool SetTypeSpecType(TST T, SourceLocation Loc, const char *&PrevSpec,
                        unsigned &DiagID, Decl *Rep, bool Owned,
                        const PrintingPolicy &Policy);
   bool SetTypeSpecType(TST T, SourceLocation TagKwLoc,
                        SourceLocation TagNameLoc, const char *&PrevSpec,
                        unsigned &DiagID, ParsedType Rep,

lib/Basic/Targets.cpp

void PPCTargetInfo::getDefaultFeatures(llvm::StringMap<bool> &Features) const {
 
   Features["qpx"] = (CPU == "a2q");
 
   if (!ABI.empty())
     Features[ABI] = true;
 }
 
 bool PPCTargetInfo::hasFeature(StringRef Feature) const {
-  return Feature == "powerpc";
+  return (Feature == "powerpc" ||
+    (Feature == "vsx" && HasVSX) ||
+    (Feature == "power8-vector" && HasP8Vector));
 }
 
   
 const char * const PPCTargetInfo::GCCRegNames[] = {
   "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7",
   "r8", "r9", "r10", "r11", "r12", "r13", "r14", "r15",
   "r16", "r17", "r18", "r19", "r20", "r21", "r22", "r23",
   "r24", "r25", "r26", "r27", "r28", "r29", "r30", "r31",

lib/Headers/altivec.h

 vec_max(vector unsigned int __a, vector bool int __b)
 {
   return __builtin_altivec_vmaxuw(__a, (vector unsigned int)__b);
 }
 
 static vector float __ATTRS_o_ai
 vec_max(vector float __a, vector float __b)
 {
+#ifdef __VSX__
+  return __builtin_vsx_xvmaxsp(__a, __b);
+#else
   return __builtin_altivec_vmaxfp(__a, __b);
+#endif
+}
+
+#ifdef __VSX__
+static vector double __ATTRS_o_ai
+vec_max(vector double __a, vector double __b)
+{
+  return __builtin_vsx_xvmaxdp(__a, __b);
 }
+#endif
 
 /* vec_vmaxsb */
 
 static vector signed char __ATTRS_o_ai
 vec_vmaxsb(vector signed char __a, vector signed char __b)
 {
   return __builtin_altivec_vmaxsb(__a, __b);
 }
vec_vmaxuw(vector unsigned int __a, vector bool int __b)
   return __builtin_altivec_vmaxuw(__a, (vector unsigned int)__b);
 }
 
 /* vec_vmaxfp */
 
 static vector float __attribute__((__always_inline__))
 vec_vmaxfp(vector float __a, vector float __b)
 {
+#ifdef __VSX__
+  return __builtin_vsx_xvmaxsp(__a, __b);
+#else
   return __builtin_altivec_vmaxfp(__a, __b);
+#endif
 }
 
 /* vec_mergeh */
 
 static vector signed char __ATTRS_o_ai
 vec_mergeh(vector signed char __a, vector signed char __b)
 {
   return vec_perm(__a, __b, (vector unsigned char)

 vec_min(vector unsigned int __a, vector bool int __b)
 {
   return __builtin_altivec_vminuw(__a, (vector unsigned int)__b);
 }
 
 static vector float __ATTRS_o_ai
 vec_min(vector float __a, vector float __b)
 {
+#ifdef __VSX__
+  return __builtin_vsx_xvminsp(__a, __b);
+#else
   return __builtin_altivec_vminfp(__a, __b);
+#endif
+}
+
+#ifdef __VSX__
+static vector double __ATTRS_o_ai
+vec_min(vector double __a, vector double __b)
+{
+  return __builtin_vsx_xvmindp(__a, __b);
 }
+#endif
 
 /* vec_vminsb */
 
 static vector signed char __ATTRS_o_ai
 vec_vminsb(vector signed char __a, vector signed char __b)
 {
   return __builtin_altivec_vminsb(__a, __b);
 }
vec_vminuw(vector unsigned int __a, vector bool int __b)
   return __builtin_altivec_vminuw(__a, (vector unsigned int)__b);
 }
 
 /* vec_vminfp */
 
 static vector float __attribute__((__always_inline__))
 vec_vminfp(vector float __a, vector float __b)
 {
+#ifdef __VSX__
+  return __builtin_vsx_xvminsp(__a, __b);
+#else
   return __builtin_altivec_vminfp(__a, __b);
+#endif
 }
 
 /* vec_mladd */
 
 #define __builtin_altivec_vmladduhm vec_mladd
 
 static vector short __ATTRS_o_ai
 vec_mladd(vector short __a, vector short __b, vector short __c)

lib/Parse/ParseDecl.cpp

case tok::kw_half:
                                      DiagID, Policy);
       break;
     case tok::kw_float:
       isInvalid = DS.SetTypeSpecType(DeclSpec::TST_float, Loc, PrevSpec,
                                      DiagID, Policy);
       break;
     case tok::kw_double:
       isInvalid = DS.SetTypeSpecType(DeclSpec::TST_double, Loc, PrevSpec,
-                                     DiagID, Policy);
+                                     DiagID, Policy, getTargetInfo());
       break;
     case tok::kw_wchar_t:
       isInvalid = DS.SetTypeSpecType(DeclSpec::TST_wchar, Loc, PrevSpec,
                                      DiagID, Policy);
       break;
     case tok::kw_char16_t:
       isInvalid = DS.SetTypeSpecType(DeclSpec::TST_char16, Loc, PrevSpec,
                                      DiagID, Policy);

lib/Sema/DeclSpec.cpp

 
 #include "clang/Sema/DeclSpec.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/LangOptions.h"
+#include "clang/Basic/TargetInfo.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Parse/ParseDiagnostic.h" // FIXME: remove this back-dependency!
 #include "clang/Sema/LocInfoType.h"
 #include "clang/Sema/ParsedTemplate.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaDiagnostic.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
bool DeclSpec::SetTypeSpecType(TST T, SourceLocation Loc,
   TSTLoc = Loc;
   TSTNameLoc = Loc;
   if (TypeAltiVecVector && (T == TST_bool) && !TypeAltiVecBool) {
     TypeAltiVecBool = true;
     return false;
   }
   TypeSpecType = T;
   TypeSpecOwned = false;
   if (TypeAltiVecVector && !TypeAltiVecBool && (TypeSpecType == TST_double)) {
     PrevSpec = DeclSpec::getSpecifierName((TST) TypeSpecType, Policy);
     DiagID = diag::err_invalid_vector_decl_spec;
     return true;
   }
   return false;
 }
 
+bool DeclSpec::SetTypeSpecType(TST T, SourceLocation Loc,
+                               const char *&PrevSpec,
+                               unsigned &DiagID,
+                               const PrintingPolicy &Policy,
+                               const TargetInfo& TI) {
+  assert(!isDeclRep(T) && !isTypeRep(T) && !isExprRep(T) &&
+         "rep required for these type-spec kinds!");
+  if (TypeSpecType != TST_unspecified) {
+    PrevSpec = DeclSpec::getSpecifierName((TST) TypeSpecType, Policy);
+    DiagID = diag::err_invalid_decl_spec_combination;
+    return true;
+  }
+  TSTLoc = Loc;
+  TSTNameLoc = Loc;
+  if (TypeAltiVecVector && (T == TST_bool) && !TypeAltiVecBool) {
+    TypeAltiVecBool = true;
+    return false;
+  }
+  TypeSpecType = T;
+  TypeSpecOwned = false;
+  bool HasVSX = TI.hasFeature("vsx");
+  if (TypeAltiVecVector && !TypeAltiVecBool && !HasVSX &&
+      (TypeSpecType == TST_double)) {
+    PrevSpec = DeclSpec::getSpecifierName((TST) TypeSpecType, Policy);
+    DiagID = diag::err_invalid_vector_decl_spec;
+    return true;
+  }
+  if (TypeAltiVecVector && !TypeAltiVecBool && HasVSX &&
+      (TypeSpecType == TST_double) &&
+      (TypeSpecWidth == TSW_long || TypeSpecWidth == TSW_longlong)) {
+    PrevSpec = DeclSpec::getSpecifierName((TST) TypeSpecType, Policy);
+    DiagID = diag::err_invalid_vector_long_double_decl_spec;
+    return true;
+  }
+  return false;
+}
+
 bool DeclSpec::SetTypeAltiVecVector(bool isAltiVecVector, SourceLocation Loc,
                           const char *&PrevSpec, unsigned &DiagID,
                           const PrintingPolicy &Policy) {
   if (TypeSpecType != TST_unspecified) {
     PrevSpec = DeclSpec::getSpecifierName((TST) TypeSpecType, Policy);
     DiagID = diag::err_invalid_vector_decl_spec_combination;
     return true;
   }

test/CodeGen/builtins-ppc-vsx.c

+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -faltivec -target-feature +vsx -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+
+vector float vf = { -1.5, 2.5, -3.5, 4.5 };
+vector double vd = { 3.5, -7.5 };
+double d = 23.4;
+
+vector float res_vf;
+vector double res_vd;
+double res_d;
+
+void test1() {
+// CHECK-LABEL: define void @test1
+
+  /* vec_max */
+  res_vf = vec_max(vf, vf);
+// CHECK: @llvm.ppc.vsx.xvmaxsp
+
+  res_vd = vec_max(vd, vd);
+// CHECK: @llvm.ppc.vsx.xvmaxdp
+
+  res_vf = vec_vmaxfp(vf, vf);
+// CHECK: @llvm.ppc.vsx.xvmaxsp
+
+  /* vec_min */
+  res_vf = vec_min(vf, vf);
+// CHECK: @llvm.ppc.vsx.xvminsp
+
+  res_vd = vec_min(vd, vd);
+// CHECK: @llvm.ppc.vsx.xvmindp
+
+  res_vf = vec_vminfp(vf, vf);
+// CHECK: @llvm.ppc.vsx.xvminsp
+
+  res_d = __builtin_vsx_xsmaxdp(d, d);
+// CHECK: @llvm.ppc.vsx.xsmaxdp
+
+  res_d = __builtin_vsx_xsmindp(d, d);
+// CHECK: @llvm.ppc.vsx.xsmindp
+}

test/Parser/vsx.c

+// RUN: %clang_cc1 -triple=powerpc64-unknown-linux-gnu -faltivec -target-feature +vsx -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple=powerpc64le-unknown-linux-gnu -faltivec -target-feature +vsx -fsyntax-only -verify %s
+
+// Legitimate for VSX.
+__vector double vv_d1;
+vector double v_d2;
+
+// These should have errors.
+__vector long double  vv_ld3;        // expected-warning {{Use of 'long' with '__vector' is deprecated}} expected-error {{cannot use 'long double' with '__vector'}}
+vector long double  v_ld4;           // expected-warning {{Use of 'long' with '__vector' is deprecated}} expected-error {{cannot use 'long double' with '__vector'}}