[TargetInfo] Sort target features before passing them to the backend

[efriedma]

Summary

Passing the features in random order will lead to unpredictable results when some of the features are related (like the architecture-version feature on ARM).

It might be possible to fix this particular case in the ARM target code, to avoid adding overlapping target features. But we should probably be sorting in any case: the behavior shouldn't depend on StringMap's hashing algorithm.

[efriedma]

Add REQUIRES line to testcase.

[fhahn]

Thanks Eli, LGTM!

lib/Basic/Targets.cpp

641 ↗

(On Diff #143821)

Could you add a comment here briefly explaining why we sort here?

cfe/trunk/lib/Basic/Targets.cpp

TargetInfo::CreateTargetInfo(DiagnosticsEngine &Diags,
   if (!Target->initFeatureMap(Features, Diags, Opts->CPU,
                               Opts->FeaturesAsWritten))
     return nullptr;
 
   // Add the features to the compile options.
   Opts->Features.clear();
   for (const auto &F : Features)
     Opts->Features.push_back((F.getValue() ? "+" : "-") + F.getKey().str());
+  // Sort here, so we handle the features in a predictable order. (This matters
+  // when we're dealing with features that overlap.)
+  llvm::sort(Opts->Features.begin(), Opts->Features.end());
 
   if (!Target->handleTargetFeatures(Opts->Features, Diags))
     return nullptr;
 
   Target->setSupportedOpenCLOpts();
   Target->setOpenCLExtensionOpts();
   Target->setMaxAtomicWidth();
 
   if (!Target->validateTarget(Diags))
     return nullptr;
 
   return Target.release();
 }

cfe/trunk/test/CodeGen/arm-build-attributes.c

+// RUN: %clang --target=arm-none-eabi -x c - -o - -S < %s -mcpu=cortex-a5 -mfpu=vfpv4-d16 | FileCheck %s
+// REQUIRES: arm-registered-target
+// CHECK: .fpu vfpv4-d16
+void foo() {}