[NFC][PowerPC] Refactor ppcUserFeaturesCheck()

[lei]

Summary

This function keeps growing, refactor to use lambda.

[lei]

This comment was removed by lei.

[lei]

This comment was removed by lei.

[lei]

updated.

[nemanjai]

LGTM. Thanks for refactoring this.

[jsji]

clang/lib/Basic/Targets/PPC.cpp

240

Looks like this is will cause behavior change.. And NOT NFC at all..

Before refactor, we will return false when *ANY* of the subfeature is ON.
After refactor, we will only return false, when *ALL* of the subfeature are ON, Passed will be set to true when any of subfeature are off .

Can you please double check?

[lei]

clang/lib/Basic/Targets/PPC.cpp

240

You are right... will post an update.

[lei]

Address review comments

[jsji]

LGTM. Since there is an improvement for multi errors, we should remove [NFC] in title and commit messages. Thanks.

clang/lib/Basic/Targets/PPC.cpp

 // - direct-move
 // - float128
 // - power9-vector
 // then go ahead and error since the customer has expressed an incompatible
 // set of options.
 static bool ppcUserFeaturesCheck(DiagnosticsEngine &Diags,
                                  const std::vector<std::string> &FeaturesVec) {
 
-  if (llvm::find(FeaturesVec, "-vsx") != FeaturesVec.end()) {
-    if (llvm::find(FeaturesVec, "+power8-vector") != FeaturesVec.end()) {
-      Diags.Report(diag::err_opt_not_valid_with_opt) << "-mpower8-vector"
-                                                     << "-mno-vsx";
-      return false;
-    }
+  // vsx was not explicitly turned off.
+  if (llvm::find(FeaturesVec, "-vsx") == FeaturesVec.end())
+    return true;
 
-    if (llvm::find(FeaturesVec, "+direct-move") != FeaturesVec.end()) {
-      Diags.Report(diag::err_opt_not_valid_with_opt) << "-mdirect-move"
-                                                     << "-mno-vsx";
-      return false;
+  auto FindVSXSubfeature = [&](StringRef Feature, StringRef Option) {
+    if (llvm::find(FeaturesVec, Feature) != FeaturesVec.end()) {
+      Diags.Report(diag::err_opt_not_valid_with_opt) << Option << "-mno-vsx";
+      return true;
     }
-
-    if (llvm::find(FeaturesVec, "+float128") != FeaturesVec.end()) {
-      Diags.Report(diag::err_opt_not_valid_with_opt) << "-mfloat128"
-                                                     << "-mno-vsx";
-      return false;
-    }
+  };
 
-    if (llvm::find(FeaturesVec, "+power9-vector") != FeaturesVec.end()) {
-      Diags.Report(diag::err_opt_not_valid_with_opt) << "-mpower9-vector"
-                                                     << "-mno-vsx";
-      return false;
-    }
-  }
+  bool Found = FindVSXSubfeature("+power8-vector", "-mpower8-vector");
+  Found |= FindVSXSubfeature("+direct-move", "-mdirect-move");
+  Found |= FindVSXSubfeature("+float128", "-mfloat128");
+  Found |= FindVSXSubfeature("+power9-vector", "-mpower9-vector");
 
-  return true;
+  // Return false if any vsx subfeatures was found.
+  return !Found;
 }
 
 bool PPCTargetInfo::initFeatureMap(
     llvm::StringMap<bool> &Features, DiagnosticsEngine &Diags, StringRef CPU,
     const std::vector<std::string> &FeaturesVec) const {
   Features["altivec"] = llvm::StringSwitch<bool>(CPU)
                             .Case("7400", true)
                             .Case("g4", true)

clang/test/Driver/ppc-dependent-options.cpp

 // RUN: not %clang -target powerpc64le-unknown-unknown -fsyntax-only \
 // RUN: -mcpu=power9 -std=c++11 -mno-vsx -mpower9-vector %s 2>&1 | \
 // RUN: FileCheck %s -check-prefix=CHECK-NVSX-P9V
 
 // RUN: not %clang -target powerpc64le-unknown-unknown -fsyntax-only \
 // RUN: -mcpu=power9 -std=c++11 -mno-vsx -mfloat128 %s 2>&1 | \
 // RUN: FileCheck %s -check-prefix=CHECK-NVSX-FLT128
 
+// RUN: not %clang -target powerpc64le-unknown-unknown -fsyntax-only \
+// RUN: -mcpu=power9 -std=c++11 -mno-vsx -mfloat128 -mpower9-vector %s 2>&1 | \
+// RUN: FileCheck %s -check-prefix=CHECK-NVSX-MULTI
+
 #ifdef __VSX__
 static_assert(false, "VSX enabled");
 #endif
 
 #ifdef __POWER8_VECTOR__
 static_assert(false, "P8V enabled");
 #endif
 
 #ifdef __POWER9_VECTOR__
 static_assert(false, "P9V enabled");
 #endif
 
 #if !defined(__VSX__) && !defined(__POWER8_VECTOR__) && \
     !defined(__POWER9_VECTOR__)
 static_assert(false, "Neither enabled");
 #endif
 
 // CHECK-DEFAULT: VSX enabled
 // CHECK-DEFAULT: P8V enabled
 // CHECK-DEFAULT-P9: P9V enabled
 // CHECK-NVSX-P8V: error: option '-mpower8-vector' cannot be specified with '-mno-vsx'
 // CHECK-NVSX-P9V: error: option '-mpower9-vector' cannot be specified with '-mno-vsx'
 // CHECK-NVSX-FLT128: error: option '-mfloat128' cannot be specified with '-mno-vsx'
 // CHECK-NVSX-DMV: error: option '-mdirect-move' cannot be specified with '-mno-vsx'
+// CHECK-NVSX-MULTI: error: option '-mfloat128' cannot be specified with '-mno-vsx'
+// CHECK-NVSX-MULTI: error: option '-mpower9-vector' cannot be specified with '-mno-vsx'
 // CHECK-NVSX: Neither enabled
 // CHECK-VSX: VSX enabled