[RISCV] Add '32bit' feature to rv32 only builtins.

[craig.topper]

Summary

The backend now has a 32bit feature as part of the recent mtune
patch. We can now use that make our rv32-only builtin error checking
work the same way as rv64-only errors.

[luismarques]

Overall LGTM.

I have one concern, though. The old error message was more user friendly. Referring to RV32 as an extension is... weird. You're already massaging the error with the OF = "RV64" / OF = "RV32". Can't you special case this feature check error handling to make it print something more like the err_32_bit_builtin_64_bit_tgt message, "this builtin is only available on 32-bit targets"? (if so, the same goes for the 64-bit case).

Also, arguably this could be two separate patches, but maybe it's not worth splitting...?

clang/lib/Sema/SemaChecking.cpp

4362

That tablegen def is still being used for X86. Maybe you could make a similar patch for X86?

[craig.topper]

clang/lib/Sema/SemaChecking.cpp

4362

There's no "32bit" feature in X86.td in the backend. This was possible for RISC-V because I added "32bit" when I refactored mtune recently.

[kito-cheng]

LGTM, and +1 for the error message improvement, but could be separated patch.

clang/include/clang/Basic/BuiltinsRISCV.def

 // Zbt extension
 TARGET_BUILTIN(__builtin_riscv_fsl_32, "LiLiLiLi", "nc", "experimental-zbt")
 TARGET_BUILTIN(__builtin_riscv_fsr_32, "LiLiLiLi", "nc", "experimental-zbt")
 TARGET_BUILTIN(__builtin_riscv_fsl_64, "WiWiWiWi", "nc", "experimental-zbt,64bit")
 TARGET_BUILTIN(__builtin_riscv_fsr_64, "WiWiWiWi", "nc", "experimental-zbt,64bit")
 
 // Zbkb extension
 TARGET_BUILTIN(__builtin_riscv_brev8, "LiLi", "nc", "zbkb")
-TARGET_BUILTIN(__builtin_riscv_zip_32, "ZiZi", "nc", "zbkb")
-TARGET_BUILTIN(__builtin_riscv_unzip_32, "ZiZi", "nc", "zbkb")
+TARGET_BUILTIN(__builtin_riscv_zip_32, "ZiZi", "nc", "zbkb,32bit")
+TARGET_BUILTIN(__builtin_riscv_unzip_32, "ZiZi", "nc", "zbkb,32bit")
 
 // Zknd extension
-TARGET_BUILTIN(__builtin_riscv_aes32dsi_32, "ZiZiZiIUc", "nc", "zknd")
-TARGET_BUILTIN(__builtin_riscv_aes32dsmi_32, "ZiZiZiIUc", "nc", "zknd")
+TARGET_BUILTIN(__builtin_riscv_aes32dsi_32, "ZiZiZiIUc", "nc", "zknd,32bit")
+TARGET_BUILTIN(__builtin_riscv_aes32dsmi_32, "ZiZiZiIUc", "nc", "zknd,32bit")
 TARGET_BUILTIN(__builtin_riscv_aes64ds_64, "WiWiWi", "nc", "zknd,64bit")
 TARGET_BUILTIN(__builtin_riscv_aes64dsm_64, "WiWiWi", "nc", "zknd,64bit")
 TARGET_BUILTIN(__builtin_riscv_aes64im_64, "WiWi", "nc", "zknd,64bit")
 
 // Zknd & zkne
 TARGET_BUILTIN(__builtin_riscv_aes64ks1i_64, "WiWiIUi", "nc", "zknd|zkne,64bit")
 TARGET_BUILTIN(__builtin_riscv_aes64ks2_64, "WiWiWi", "nc", "zknd|zkne,64bit")
 
 // Zkne extension
-TARGET_BUILTIN(__builtin_riscv_aes32esi_32, "ZiZiZiIUc", "nc", "zkne")
-TARGET_BUILTIN(__builtin_riscv_aes32esmi_32, "ZiZiZiIUc", "nc", "zkne")
+TARGET_BUILTIN(__builtin_riscv_aes32esi_32, "ZiZiZiIUc", "nc", "zkne,32bit")
+TARGET_BUILTIN(__builtin_riscv_aes32esmi_32, "ZiZiZiIUc", "nc", "zkne,32bit")
 TARGET_BUILTIN(__builtin_riscv_aes64es_64, "WiWiWi", "nc", "zkne,64bit")
 TARGET_BUILTIN(__builtin_riscv_aes64esm_64, "WiWiWi", "nc", "zkne,64bit")
 
 // Zknh extension
 TARGET_BUILTIN(__builtin_riscv_sha256sig0, "LiLi", "nc", "zknh")
 TARGET_BUILTIN(__builtin_riscv_sha256sig1, "LiLi", "nc", "zknh")
 TARGET_BUILTIN(__builtin_riscv_sha256sum0, "LiLi", "nc", "zknh")
 TARGET_BUILTIN(__builtin_riscv_sha256sum1, "LiLi", "nc", "zknh")
 
-TARGET_BUILTIN(__builtin_riscv_sha512sig0h_32, "ZiZiZi", "nc", "zknh")
-TARGET_BUILTIN(__builtin_riscv_sha512sig0l_32, "ZiZiZi", "nc", "zknh")
-TARGET_BUILTIN(__builtin_riscv_sha512sig1h_32, "ZiZiZi", "nc", "zknh")
-TARGET_BUILTIN(__builtin_riscv_sha512sig1l_32, "ZiZiZi", "nc", "zknh")
-TARGET_BUILTIN(__builtin_riscv_sha512sum0r_32, "ZiZiZi", "nc", "zknh")
-TARGET_BUILTIN(__builtin_riscv_sha512sum1r_32, "ZiZiZi", "nc", "zknh")
+TARGET_BUILTIN(__builtin_riscv_sha512sig0h_32, "ZiZiZi", "nc", "zknh,32bit")
+TARGET_BUILTIN(__builtin_riscv_sha512sig0l_32, "ZiZiZi", "nc", "zknh,32bit")
+TARGET_BUILTIN(__builtin_riscv_sha512sig1h_32, "ZiZiZi", "nc", "zknh,32bit")
+TARGET_BUILTIN(__builtin_riscv_sha512sig1l_32, "ZiZiZi", "nc", "zknh,32bit")
+TARGET_BUILTIN(__builtin_riscv_sha512sum0r_32, "ZiZiZi", "nc", "zknh,32bit")
+TARGET_BUILTIN(__builtin_riscv_sha512sum1r_32, "ZiZiZi", "nc", "zknh,32bit")
 TARGET_BUILTIN(__builtin_riscv_sha512sig0_64, "WiWi", "nc", "zknh,64bit")
 TARGET_BUILTIN(__builtin_riscv_sha512sig1_64, "WiWi", "nc", "zknh,64bit")
 TARGET_BUILTIN(__builtin_riscv_sha512sum0_64, "WiWi", "nc", "zknh,64bit")
 TARGET_BUILTIN(__builtin_riscv_sha512sum1_64, "WiWi", "nc", "zknh,64bit")
 
 // Zksed extension
 TARGET_BUILTIN(__builtin_riscv_sm4ed, "LiLiLiIUc", "nc", "zksed")
 TARGET_BUILTIN(__builtin_riscv_sm4ks, "LiLiLiIUc", "nc", "zksed")
 
 // Zksh extension
 TARGET_BUILTIN(__builtin_riscv_sm3p0, "LiLi", "nc", "zksh")
 TARGET_BUILTIN(__builtin_riscv_sm3p1, "LiLi", "nc", "zksh")
 
 #undef BUILTIN
 #undef TARGET_BUILTIN

clang/lib/Basic/Targets/RISCV.cpp

bool RISCVTargetInfo::initFeatureMap(
     llvm::StringMap<bool> &Features, DiagnosticsEngine &Diags, StringRef CPU,
     const std::vector<std::string> &FeaturesVec) const {
 
   unsigned XLen = 32;
 
   if (getTriple().getArch() == llvm::Triple::riscv64) {
     Features["64bit"] = true;
     XLen = 64;
+  } else {
+    Features["32bit"] = true;
   }
 
   auto ParseResult = llvm::RISCVISAInfo::parseFeatures(XLen, FeaturesVec);
   if (!ParseResult) {
     std::string Buffer;
     llvm::raw_string_ostream OutputErrMsg(Buffer);
     handleAllErrors(ParseResult.takeError(), [&](llvm::StringError &ErrMsg) {
       OutputErrMsg << ErrMsg.getMessage();

 
 /// Return true if has this feature, need to sync with handleTargetFeatures.
 bool RISCVTargetInfo::hasFeature(StringRef Feature) const {
   bool Is64Bit = getTriple().getArch() == llvm::Triple::riscv64;
   auto Result = llvm::StringSwitch<Optional<bool>>(Feature)
                     .Case("riscv", true)
                     .Case("riscv32", !Is64Bit)
                     .Case("riscv64", Is64Bit)
+                    .Case("32bit", !Is64Bit)
                     .Case("64bit", Is64Bit)
                     .Default(None);
   if (Result)
     return Result.value();
 
   if (ISAInfo->isSupportedExtensionFeature(Feature))
     return ISAInfo->hasExtension(Feature);
 

clang/lib/Sema/SemaChecking.cpp

bool Sema::CheckRISCVLMUL(CallExpr *TheCall, unsigned ArgNum) {
   int64_t Val = Result.getSExtValue();
   if ((Val >= 0 && Val <= 3) || (Val >= 5 && Val <= 7))
     return false;
 
   return Diag(TheCall->getBeginLoc(), diag::err_riscv_builtin_invalid_lmul)
          << Arg->getSourceRange();
 }
 
-static bool isRISCV32Builtin(unsigned BuiltinID) {
-  // These builtins only work on riscv32 targets.
-  switch (BuiltinID) {
-  case RISCV::BI__builtin_riscv_zip_32:
-  case RISCV::BI__builtin_riscv_unzip_32:
-  case RISCV::BI__builtin_riscv_aes32dsi_32:
-  case RISCV::BI__builtin_riscv_aes32dsmi_32:
-  case RISCV::BI__builtin_riscv_aes32esi_32:
-  case RISCV::BI__builtin_riscv_aes32esmi_32:
-  case RISCV::BI__builtin_riscv_sha512sig0h_32:
-  case RISCV::BI__builtin_riscv_sha512sig0l_32:
-  case RISCV::BI__builtin_riscv_sha512sig1h_32:
-  case RISCV::BI__builtin_riscv_sha512sig1l_32:
-  case RISCV::BI__builtin_riscv_sha512sum0r_32:
-  case RISCV::BI__builtin_riscv_sha512sum1r_32:
-    return true;
-  }
-
-  return false;
-}
-
 bool Sema::CheckRISCVBuiltinFunctionCall(const TargetInfo &TI,
                                          unsigned BuiltinID,
                                          CallExpr *TheCall) {
   // CodeGenFunction can also detect this, but this gives a better error
   // message.
   bool FeatureMissing = false;
   SmallVector<StringRef> ReqFeatures;
   StringRef Features = Context.BuiltinInfo.getRequiredFeatures(BuiltinID);
   Features.split(ReqFeatures, ',');
 
-  // Check for 32-bit only builtins on a 64-bit target.
-  const llvm::Triple &TT = TI.getTriple();
-  if (TT.getArch() != llvm::Triple::riscv32 && isRISCV32Builtin(BuiltinID))
-    return Diag(TheCall->getCallee()->getBeginLoc(),
-                diag::err_32_bit_builtin_64_bit_tgt);
-
   // Check if each required feature is included
   for (StringRef F : ReqFeatures) {
     SmallVector<StringRef> ReqOpFeatures;
     F.split(ReqOpFeatures, '|');
     bool HasFeature = false;
     for (StringRef OF : ReqOpFeatures) {
       if (TI.hasFeature(OF)) {
         HasFeature = true;
         continue;
       }
     }
 
     if (!HasFeature) {
       std::string FeatureStrs;
       for (StringRef OF : ReqOpFeatures) {
         // If the feature is 64bit, alter the string so it will print better in
         // the diagnostic.
         if (OF == "64bit")
           OF = "RV64";
+        if (OF == "32bit")
+          OF = "RV32";
 
         // Convert features like "zbr" and "experimental-zbr" to "Zbr".
         OF.consume_front("experimental-");
         std::string FeatureStr = OF.str();
         FeatureStr[0] = std::toupper(FeatureStr[0]);
         // Combine strings.
         FeatureStrs += FeatureStrs == "" ? "" : ", ";
         FeatureStrs += "'";

clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbkb-error.c

+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple riscv64 -target-feature +zbkb -verify %s -o -
+
+int zip(int rs1)
+{
+  return __builtin_riscv_zip_32(rs1); // expected-error {{builtin requires at least one of the following extensions support to be enabled : 'RV32'}}
+}
+
+int unzip(int rs1)
+{
+  return __builtin_riscv_unzip_32(rs1); // expected-error {{builtin requires at least one of the following extensions support to be enabled : 'RV32'}}
+}

clang/test/Driver/riscv-default-features.c

 // RUN: %clang --target=riscv32-unknown-elf -S -emit-llvm %s -o - | FileCheck %s -check-prefix=RV32
 // RUN: %clang --target=riscv64-unknown-elf -S -emit-llvm %s -o - | FileCheck %s -check-prefix=RV64
 
-// RV32: "target-features"="+a,+c,+m,+relax,-save-restore"
+// RV32: "target-features"="+32bit,+a,+c,+m,+relax,-save-restore"
 // RV64: "target-features"="+64bit,+a,+c,+m,+relax,-save-restore"
 
 // Dummy function
 int foo(void){
   return  3;
 }