[ARM] The Driver does not set the +strict-align flag when targeting armv6m + netbsd

[labrinea]

Summary

Tests are misssing for armv6m + darwin as well.

[rengolin]

lib/Basic/Targets.cpp

4456

Why is this not necessary any more?

[labrinea]

lib/Basic/Targets.cpp

4456

The logic is still necessary for the back-end to emit aligned loads and stores, but it should be moved to the Driver, where +strict-align is set for all arch+OS combinations as shown below.

[rengolin]

lib/Basic/Targets.cpp

4456

Oh, so this ended up as a work-around for the correct change that you just did, below?

[labrinea]

lib/Basic/Targets.cpp

4456

Apparently yes. The front-end checks this flag in order to decide whether it should define the __ARM_FEATURE_UNALIGNED macro, and the back-end needs it to chose the correct instruction lowering for memory accesses. The best place to set this flag is in Driver, where all target features are handled.

[rengolin]

Absolutely agree. If we miss some other target, we should add it there, too, not cover later. LGTM. Thanks!

lib/Basic/Targets.cpp

bool initFeatureMap(llvm::StringMap<bool> &Features, DiagnosticsEngine &Diags,
     // get default Extension features
     unsigned Extensions = llvm::ARM::getDefaultExtensions(CPU);
     llvm::ARM::getExtensionFeatures(Extensions, TargetFeatures);
 
     for (const char *Feature : TargetFeatures)
       if (Feature[0] == '+')
         Features[Feature+1] = true; 
 
-    if (ArchVersion < 6  || 
-       (ArchVersion == 6 && ArchProfile == llvm::ARM::PK_M))
-      Features["strict-align"] = true;
-
     return TargetInfo::initFeatureMap(Features, Diags, CPU, FeaturesVec);
   }
 
   bool handleTargetFeatures(std::vector<std::string> &Features,
                             DiagnosticsEngine &Diags) override {
     FPU = 0;
     CRC = 0;
     Crypto = 0;

lib/Driver/Tools.cpp

else if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
     // which raises an alignment fault on unaligned accesses. Linux
     // defaults this bit to 0 and handles it as a system-wide (not
     // per-process) setting. It is therefore safe to assume that ARMv7+
     // Linux targets support unaligned accesses. The same goes for NaCl.
     //
     // The above behavior is consistent with GCC.
     int VersionNum = getARMSubArchVersionNumber(Triple);
     if (Triple.isOSDarwin() || Triple.isOSNetBSD()) {
-      if (VersionNum < 6)
+      if (VersionNum < 6 ||
+          Triple.getSubArch() == llvm::Triple::SubArchType::ARMSubArch_v6m)
         Features.push_back("+strict-align");
     } else if (Triple.isOSLinux() || Triple.isOSNaCl()) {
       if (VersionNum < 7)
         Features.push_back("+strict-align");
     } else
       Features.push_back("+strict-align");
   }
 

test/Driver/arm-alignment.c

 // RUN: FileCheck --check-prefix=CHECK-ALIGNED-ARM < %t %s
 
 // RUN: %clang -target armv6-unknown-linux -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-ALIGNED-ARM < %t %s
 
 // RUN: %clang -target armv6-unknown-nacl-gnueabihf -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-ALIGNED-ARM < %t %s
 
+// RUN: %clang -target armv6m-apple-darwin -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-ALIGNED-ARM < %t %s
+
+// RUN: %clang -target armv6m-netbsd-eabi -### %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-ALIGNED-ARM < %t %s
+
 // RUN: %clang -target aarch64-none-gnueabi -mno-unaligned-access -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-ALIGNED-AARCH64 < %t %s
 
 // RUN: %clang -target aarch64-none-gnueabi -mstrict-align -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-ALIGNED-AARCH64 < %t %s
 
 // RUN: %clang -target aarch64-none-gnueabi -munaligned-access -mno-unaligned-access -### %s 2> %t
 // RUN: FileCheck --check-prefix=CHECK-ALIGNED-AARCH64 < %t %s