[PPC] Move argument range checks for HTM and crypto builtins to Sema

[kbarton]

Summary

The argument range checks for the HTM and Crypto builtins were implemented in CGBuiltin.cpp, not in Sema. This change moves them to the appropriate location in SemaChecking.cpp. It requires the creation of a new method in the Sema class to do checks for PPC-specific builtins.

[wschmidt]

One stylistic comment from me; otherwise looks just fine. I'll defer to Richard as he has much more expertise here.

lib/Sema/SemaChecking.cpp

913

I don't particularly like this style of putting the code on the same line as the case (I realize it fits in with the surrounding code). It obfuscates that this code is shared by tbegin and tend, for example.

[rsmith]

Some tiny nits, but this looks basically fine to me too. Thanks for following up on this!

lib/Sema/SemaChecking.cpp

910–911

Parens not necessary here.

919–920

... or here.

[kbarton]

Committed revision 233586.

include/clang/Sema/Sema.h

private:
   bool CheckARMBuiltinExclusiveCall(unsigned BuiltinID, CallExpr *TheCall,
                                     unsigned MaxWidth);
   bool CheckNeonBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
   bool CheckARMBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
 
   bool CheckAArch64BuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
   bool CheckMipsBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
   bool CheckX86BuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
+  bool CheckPPCBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall);
   
   bool SemaBuiltinVAStart(CallExpr *TheCall);
   bool SemaBuiltinVAStartARM(CallExpr *Call);
   bool SemaBuiltinUnorderedCompare(CallExpr *TheCall);
   bool SemaBuiltinFPClassification(CallExpr *TheCall, unsigned NumArgs);
 
 public:
   // Used by C++ template instantiation.
   ExprResult SemaBuiltinShuffleVector(CallExpr *TheCall);

lib/CodeGen/CGBuiltin.cpp

case PPC::BI__builtin_vsx_stxvd2x:
       break;
     case PPC::BI__builtin_vsx_stxvw4x:
       ID = Intrinsic::ppc_vsx_stxvw4x;
       break;
     }
     llvm::Function *F = CGM.getIntrinsic(ID);
     return Builder.CreateCall(F, Ops, "");
   }
-
-  // P8 Crypto builtins
-  case PPC::BI__builtin_altivec_crypto_vshasigmaw:
-  case PPC::BI__builtin_altivec_crypto_vshasigmad:
-  {
-    ConstantInt *CI1 = dyn_cast<ConstantInt>(Ops[1]);
-    ConstantInt *CI2 = dyn_cast<ConstantInt>(Ops[2]);
-    assert(CI1 && CI2);
-    if (CI1->getZExtValue() > 1) {
-      CGM.Error(E->getArg(1)->getExprLoc(),
-                "argument out of range (should be 0-1).");
-      return llvm::UndefValue::get(Ops[0]->getType());
-    }
-    if (CI2->getZExtValue() > 15) {
-      CGM.Error(E->getArg(2)->getExprLoc(),
-                "argument out of range (should be 0-15).");
-      return llvm::UndefValue::get(Ops[0]->getType());
-    }
-    switch (BuiltinID) {
-    default: llvm_unreachable("Unsupported sigma intrinsic!");
-    case PPC::BI__builtin_altivec_crypto_vshasigmaw:
-      ID = Intrinsic::ppc_altivec_crypto_vshasigmaw;
-      break;
-    case PPC::BI__builtin_altivec_crypto_vshasigmad:
-      ID = Intrinsic::ppc_altivec_crypto_vshasigmad;
-      break;
-    }
-    llvm::Function *F = CGM.getIntrinsic(ID);
-    return Builder.CreateCall(F, Ops, "");
-  }
-
-  // HTM builtins
-  case PPC::BI__builtin_tbegin:
-  case PPC::BI__builtin_tend:
-  case PPC::BI__builtin_tsr: {
-    unsigned int MaxValue;
-    // The HTM instructions only accept one argument and with limited range.
-    ConstantInt *CI = dyn_cast<ConstantInt>(Ops[0]);
-    assert(CI);
-    switch (BuiltinID) {
-    case PPC::BI__builtin_tbegin:
-      ID = Intrinsic::ppc_tbegin;
-      MaxValue = 1;
-      break;
-    case PPC::BI__builtin_tend:
-      ID = Intrinsic::ppc_tend;
-      MaxValue = 1;
-      break;
-    case PPC::BI__builtin_tsr:
-      ID = Intrinsic::ppc_tsr;
-      MaxValue = 7;
-      break;
-    }
-    if (CI->getZExtValue() > MaxValue) {
-      std::stringstream ss;
-      ss << "argument out of range (should be 0 or " << MaxValue << ")";
-      CGM.Error(E->getArg(0)->getExprLoc(), ss.str());
-      return llvm::UndefValue::get(Ops[0]->getType());
-    }
-
-    llvm::Function *F = CGM.getIntrinsic(ID);
-    return Builder.CreateCall(F, Ops, "");
-  }
-  case PPC::BI__builtin_tabortdc:
-  case PPC::BI__builtin_tabortwc: {
-    // For wd and dc variant of tabort first argument must be a 5-bit constant
-    // integer
-    ConstantInt *CI = dyn_cast<ConstantInt>(Ops[0]);
-    assert(CI);
-    if (CI->getZExtValue() > 31) {
-      CGM.ErrorUnsupported(E->getArg(0), "argument out of range (should be 0-31)");
-      return llvm::UndefValue::get(Ops[0]->getType());
-    }
-    switch (BuiltinID) {
-    case PPC::BI__builtin_tabortdc:
-      ID = Intrinsic::ppc_tabortdc;
-      break;
-    case PPC::BI__builtin_tabortwc:
-      ID = Intrinsic::ppc_tabortwc;
-      break;
-    }
-    llvm::Function *F = CGM.getIntrinsic(ID);
-    return Builder.CreateCall(F, Ops, "");
-  }
-  case PPC::BI__builtin_tabortdci:
-  case PPC::BI__builtin_tabortwci: {
-    // For wd and dc variant of tabort first and third argument must be a
-    // 5-bit constant integer
-    ConstantInt *CI = dyn_cast<ConstantInt>(Ops[0]);
-    assert(CI);
-    if (CI->getZExtValue() > 31) {
-      CGM.ErrorUnsupported(E->getArg(0), "argument out of range (should be 0-31)");
-      return llvm::UndefValue::get(Ops[0]->getType());
-    }
-    CI = dyn_cast<ConstantInt>(Ops[2]);
-    assert(CI);
-    if (CI->getZExtValue() > 31) {
-      CGM.ErrorUnsupported(E->getArg(2), "argument out of range (should be 0-31)");
-      return llvm::UndefValue::get(Ops[2]->getType());
-    }
-    switch (BuiltinID) {
-    default: llvm_unreachable("Unsupported htm intrinsic!");
-    case PPC::BI__builtin_tabortdci:
-      ID = Intrinsic::ppc_tabortdci;
-      break;
-    case PPC::BI__builtin_tabortwci:
-      ID = Intrinsic::ppc_tabortwci;
-      break;
-    }
-    llvm::Function *F = CGM.getIntrinsic(ID);
-    return Builder.CreateCall(F, Ops, "");
-  }
-
   }
 }
 
 // Emit an intrinsic that has 1 float or double.
 static Value *emitUnaryFPBuiltin(CodeGenFunction &CGF,
                                  const CallExpr *E,
                                  unsigned IntrinsicID) {
   llvm::Value *Src0 = CGF.EmitScalarExpr(E->getArg(0));

lib/Sema/SemaChecking.cpp

switch (Context.getTargetInfo().getTriple().getArch()) {
         if (CheckMipsBuiltinFunctionCall(BuiltinID, TheCall))
           return ExprError();
         break;
       case llvm::Triple::x86:
       case llvm::Triple::x86_64:
         if (CheckX86BuiltinFunctionCall(BuiltinID, TheCall))
           return ExprError();
         break;
+      case llvm::Triple::ppc:
+      case llvm::Triple::ppc64:
+      case llvm::Triple::ppc64le:
+        if (CheckPPCBuiltinFunctionCall(BuiltinID, TheCall))
+          return ExprError();
+        break;
       default:
         break;
     }
   }
 
   return TheCallResult;
 }
 
bool Sema::CheckMipsBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) {
   case Mips::BI__builtin_mips_precr_sra_ph_w: i = 2; l = 0; u = 31; break;
   case Mips::BI__builtin_mips_precr_sra_r_ph_w: i = 2; l = 0; u = 31; break;
   case Mips::BI__builtin_mips_prepend: i = 2; l = 0; u = 31; break;
   }
 
   return SemaBuiltinConstantArgRange(TheCall, i, l, u);
 }
 
+bool Sema::CheckPPCBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) {
+  unsigned i = 0, l = 0, u = 0;
+  switch (BuiltinID) {
+  default: return false;
+  case PPC::BI__builtin_altivec_crypto_vshasigmaw:
+  case PPC::BI__builtin_altivec_crypto_vshasigmad:
+    return (SemaBuiltinConstantArgRange(TheCall, 1, 0, 1) ||
+            SemaBuiltinConstantArgRange(TheCall, 2, 0, 15));
+  case PPC::BI__builtin_tbegin:
+  case PPC::BI__builtin_tend: i = 0; l = 0; u = 1; break;
+  case PPC::BI__builtin_tsr: i = 0; l = 0; u = 7; break;
+  case PPC::BI__builtin_tabortwc:
+  case PPC::BI__builtin_tabortdc: i = 0; l = 0; u = 31; break;
+  case PPC::BI__builtin_tabortwci:
+  case PPC::BI__builtin_tabortdci:
+    return (SemaBuiltinConstantArgRange(TheCall, 0, 0, 31) ||
+            SemaBuiltinConstantArgRange(TheCall, 2, 0, 31));
+  }
+  return SemaBuiltinConstantArgRange(TheCall, i, l, u);
+}
+
 bool Sema::CheckX86BuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) {
   unsigned i = 0, l = 0, u = 0;
   switch (BuiltinID) {
   default: return false;
   case X86::BI_mm_prefetch: i = 1; l = 0; u = 3; break;
   case X86::BI__builtin_ia32_sha1rnds4: i = 2, l = 0; u = 3; break;
   case X86::BI__builtin_ia32_vpermil2pd:
   case X86::BI__builtin_ia32_vpermil2pd256:

test/CodeGen/builtins-ppc-crypto-diag.c

-// REQUIRES: powerpc-registered-target
-// RUN: not %clang_cc1 -faltivec -triple powerpc64le-unknown-unknown -D_T1LW -target-feature +crypto -emit-llvm %s -o - 2>&1 | FileCheck %s -check-prefix=CHECK-T1
-// RUN: not %clang_cc1 -faltivec -triple powerpc64le-unknown-unknown -D_T1MW -target-feature +crypto -emit-llvm %s -o - 2>&1 | FileCheck %s -check-prefix=CHECK-T1
-// RUN: not %clang_cc1 -faltivec -triple powerpc64le-unknown-unknown -D_T2LW -target-feature +crypto -emit-llvm %s -o - 2>&1 | FileCheck %s -check-prefix=CHECK-T2
-// RUN: not %clang_cc1 -faltivec -triple powerpc64le-unknown-unknown -D_T2MW -target-feature +crypto -emit-llvm %s -o - 2>&1 | FileCheck %s -check-prefix=CHECK-T2
-// RUN: not %clang_cc1 -faltivec -triple powerpc64le-unknown-unknown -D_T1LD -target-feature +crypto -emit-llvm %s -o - 2>&1 | FileCheck %s -check-prefix=CHECK-T1
-// RUN: not %clang_cc1 -faltivec -triple powerpc64le-unknown-unknown -D_T1MD -target-feature +crypto -emit-llvm %s -o - 2>&1 | FileCheck %s -check-prefix=CHECK-T1
-// RUN: not %clang_cc1 -faltivec -triple powerpc64le-unknown-unknown -D_T2LD -target-feature +crypto -emit-llvm %s -o - 2>&1 | FileCheck %s -check-prefix=CHECK-T2
-// RUN: not %clang_cc1 -faltivec -triple powerpc64le-unknown-unknown -D_T2MD -target-feature +crypto -emit-llvm %s -o - 2>&1 | FileCheck %s -check-prefix=CHECK-T2
-#include <altivec.h>
-
-#define W_INIT { 0x01020304, 0x05060708, \
-                  0x090A0B0C, 0x0D0E0F10 };
-#define D_INIT { 0x0102030405060708, \
-                  0x090A0B0C0D0E0F10 };
-vector unsigned int test_vshasigmaw_or(void)
-{
-  vector unsigned int a = W_INIT
-#ifdef _T1LW // Arg1 too large
-  vector unsigned int b = __builtin_crypto_vshasigmaw(a, 2, 15);
-#elif defined(_T1MW) // Arg1 negative
-  vector unsigned int c = __builtin_crypto_vshasigmaw(a, -1, 15);
-#elif defined(_T2LW) // Arg2 too large
-  vector unsigned int d = __builtin_crypto_vshasigmaw(a, 0, 85);
-#elif defined(_T2MW) // Arg1 negative
-  vector unsigned int e = __builtin_crypto_vshasigmaw(a, 1, -15);
-#endif
-  return __builtin_crypto_vshasigmaw(a, 1, 15);
-}
-
-vector unsigned long long test_vshasigmad_or(void)
-{
-  vector unsigned long long a = D_INIT
-#ifdef _T1LD // Arg1 too large
-  vector unsigned long long b = __builtin_crypto_vshasigmad(a, 2, 15);
-#elif defined(_T1MD) // Arg1 negative
-  vector unsigned long long c = __builtin_crypto_vshasigmad(a, -1, 15);
-#elif defined(_T2LD) // Arg2 too large
-  vector unsigned long long d = __builtin_crypto_vshasigmad(a, 0, 85);
-#elif defined(_T2MD) // Arg1 negative
-  vector unsigned long long e = __builtin_crypto_vshasigmad(a, 1, -15);
-#endif
-  return __builtin_crypto_vshasigmad(a, 0, 15);
-}
-
-// CHECK-T1: error: argument out of range (should be 0-1).
-// CHECK-T2: error: argument out of range (should be 0-15).

test/Sema/builtins-ppc.c

+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -faltivec -target-feature +htm                       \
+// RUN: -triple powerpc64-unknown-unknown -DTEST_HTM -fsyntax-only      \
+// RUN: -verify %s
+
+// RUN: %clang_cc1 -faltivec -target-feature +crypto                    \
+// -triple powerpc64le-unknown-unknown -DTEST_CRYPTO -fsyntax-only      \
+// -verify %s
+
+#ifdef TEST_HTM
+void test_htm() {
+  __builtin_tbegin(4); // expected-error {{argument should be a value from 0 to 1}}
+  __builtin_tend(-1);  // expected-error {{argument should be a value from 0 to 1}}
+  __builtin_tsr(55);   // expected-error {{argument should be a value from 0 to 7}}
+  __builtin_tabortwc(-5, 2, 3); // expected-error {{argument should be a value from 0 to 31}} 
+  __builtin_tabortdc(55, 2, 3); // expected-error {{argument should be a value from 0 to 31}}
+  __builtin_tabortwci(-5, 2, 5); // expected-error {{argument should be a value from 0 to 31}}
+  __builtin_tabortwci(5, 2, 55); // expected-error {{argument should be a value from 0 to 31}}  
+  __builtin_tabortdci(-5, 2, 5); // expected-error {{argument should be a value from 0 to 31}}
+  __builtin_tabortdci(5, 2, 55); // expected-error {{argument should be a value from 0 to 31}}
+}
+#endif
+
+
+#ifdef TEST_CRYPTO
+#include <altivec.h>
+
+#define W_INIT { 0x01020304, 0x05060708, 0x090A0B0C, 0x0D0E0F10 };
+#define D_INIT { 0x0102030405060708, 0x090A0B0C0D0E0F10 };
+vector unsigned int test_vshasigmaw_or(void)
+{
+  vector unsigned int a = W_INIT
+  vector unsigned int b = __builtin_crypto_vshasigmaw(a, 2, 15);  // expected-error {{argument should be a value from 0 to 1}}
+  vector unsigned int c = __builtin_crypto_vshasigmaw(a, -1, 15); // expected-error {{argument should be a value from 0 to 1}}
+  vector unsigned int d = __builtin_crypto_vshasigmaw(a, 0, 85);  // expected-error {{argument should be a value from 0 to 15}}
+  vector unsigned int e = __builtin_crypto_vshasigmaw(a, 1, -15); // expected-error {{argument should be a value from 0 to 15}}
+  return __builtin_crypto_vshasigmaw(a, 1, 15);
+}
+
+vector unsigned long long test_vshasigmad_or(void)
+{
+  vector unsigned long long a = D_INIT
+  vector unsigned long long b = __builtin_crypto_vshasigmad(a, 2, 15);  // expected-error {{argument should be a value from 0 to 1}}
+  vector unsigned long long c = __builtin_crypto_vshasigmad(a, -1, 15); // expected-error {{argument should be a value from 0 to 1}}
+  vector unsigned long long d = __builtin_crypto_vshasigmad(a, 0, 85);  // expected-error {{argument should be a value from 0 to 1}}
+  vector unsigned long long e = __builtin_crypto_vshasigmad(a, 1, -15); // expected-error {{argument should be a value from 0 to 1}}
+  return __builtin_crypto_vshasigmad(a, 0, 15);
+}
+
+#endif
+