[ARM] Allow codegen for Armv6m eXecute-Only (XO) sections

[stuij]

Summary

This patch moves the overall lower-bound arch restriction for Arm XO sections
from v8m to v6m. Actual implementation of code-gen for v6m will follow in
follow-up patches, which will include an implementation of relocations needed to
support this.

[simonwallis2]

llvm/lib/Target/ARM/ARMSubtarget.cpp

435

V6M does not have Movt.
At face value, this line looks wrong and leads to about 30 unit test fails.

[simonwallis2]

llvm/lib/Target/ARM/ARMSubtarget.cpp

435

I clarify: there are no test fails with this patch on its own.
The unit test fails I saw where when building this patch in conjunction with related patch https://reviews.llvm.org/D149443

[stuij]

llvm/lib/Target/ARM/ARMSubtarget.cpp

435

Correct, this shouldn't be here. Initially it looked like we were going to repurpose movt for v6m, but that should have been part of another patch. And now it looks like we'll handle v6m immediates separately. In any case I'll remove this.

[stuij]

addressed review comments and updated tests

[tschuett]

llvm/lib/Target/ARM/ARMSubtarget.cpp

194

What happens in release mode? At the top you now claim that ARMV6M is supported. Could hasV6MOps() silently return false?

[stuij]

addressed review comment

llvm/lib/Target/ARM/ARMSubtarget.cpp

194

This should really be caught by the frontend, but yes, I'm not sure why we use assert so often in this kind of code instead of unreachable. Changed.

[MaskRay]

clang/test/Driver/arm-execute-only.c

1

test/Driver tests should test just the Driver part, not sema/codegen, so we generally use -### to skip compilation.

-target has been deprecated since 3.x. Use --target=

[MaskRay]

llvm/lib/Target/ARM/ARMSubtarget.cpp

195

If this code path is actually reachable, a proper error reporting will be better. At the least report_fatal_error can be used. Don't use llvm_unreachable for known-reachable code paths.

[stuij]

addressed review comments

[MaskRay]

Actual implementation of code-gen for v6m will follow in follow-up patches, which will include an implementation of relocations needed to support this.

Normally we implement the feature in llvm/ first, and the clang/lib/Driver change should be the last patch.
Some users just check whether clang --target=xxx accepts a given option, not test that the feature actually works (it's sometimes non-trivial to figure out all corners as they are not compiler feature implementers.)

clang/test/Driver/arm-execute-only.c

1–6

If you don't check -cc1 options with FileCheck, you can also replace -### with -fdriver-only -Werror and drop two -NOT: patterns below. You can play with some examples to see its effect:)

If the clang -c -fdriver-only -Werror command gives no output, you can use ... 2>&1 | count 0

[stuij]

In D149444#4415306, @MaskRay wrote:

Normally we implement the feature in llvm/ first, and the clang/lib/Driver change should be the last patch.

Yes good point. I won't commit it yet. For this feature we're using Phab as a vehicle to swap patches, so we're reviewing these upstream as they become available.

clang/test/Driver/arm-execute-only.c

1–6

Very useful info. Thanks for the tips!

[stuij]

• Moved the backend parts of this patch to D149443.
• Slightly tweaked the way we call the specific arch in the tests.

[stuij]

In D149444#4425134, @stuij wrote:

• Moved the backend parts of this patch to D149443.

Wrong Phab review -^ It should be D152795.

[MaskRay]

Looks great!

clang/test/Driver/arm-execute-only.c

1–6

LGTM once the simplification is done (e.g. clang -c -fdriver-only -Werror)

[stuij]

address review comments

[stuij]

clang/test/Driver/arm-execute-only.c

1–6

yup, thanks for that. I was struggling to get rid of those NOTs.

clang/lib/Driver/ToolChains/Arch/ARM.cpp

fp16_fml_fallthrough:
   // Generate execute-only output (no data access to code sections).
   // This only makes sense for the compiler, not for the assembler.
   if (!ForAS) {
     // Supported only on ARMv6T2 and ARMv7 and above.
     // Cannot be combined with -mno-movt.
     if (Arg *A = Args.getLastArg(options::OPT_mexecute_only, options::OPT_mno_execute_only)) {
       if (A->getOption().matches(options::OPT_mexecute_only)) {
         if (getARMSubArchVersionNumber(Triple) < 7 &&
-            llvm::ARM::parseArch(Triple.getArchName()) != llvm::ARM::ArchKind::ARMV6T2)
+            llvm::ARM::parseArch(Triple.getArchName()) != llvm::ARM::ArchKind::ARMV6T2 &&
+             llvm::ARM::parseArch(Triple.getArchName()) != llvm::ARM::ArchKind::ARMV6M)
               D.Diag(diag::err_target_unsupported_execute_only) << Triple.getArchName();
         else if (Arg *B = Args.getLastArg(options::OPT_mno_movt))
           D.Diag(diag::err_opt_not_valid_with_opt)
               << A->getAsString(Args) << B->getAsString(Args);
         Features.push_back("+execute-only");
       }
     }
   }

clang/test/Driver/arm-execute-only.c

-// RUN: not %clang -c -target thumbv6m-eabi -mexecute-only %s 2>&1 | \
+// RUN: %clang -c -target thumbv6m-eabi -mexecute-only %s 2>&1 | \
+// RUN:   FileCheck -allow-empty --check-prefix CHECK-THIS %s
+// CHECK-THIS-NOT: warning:
+// CHECK-THIS-NOT: error:
+
+// RUN: not %clang -c -target thumbv6-eabi -mexecute-only %s 2>&1 |    \
 // RUN:   FileCheck --check-prefix CHECK-EXECUTE-ONLY-NOT-SUPPORTED %s
-// CHECK-EXECUTE-ONLY-NOT-SUPPORTED: error: execute only is not supported for the thumbv6m sub-architecture
+// CHECK-EXECUTE-ONLY-NOT-SUPPORTED: error: execute only is not supported for the armv6 sub-architecture
 
 // RUN: not %clang -target armv8m.main-eabi -mexecute-only -mno-movt %s 2>&1 \
 // RUN:    | FileCheck %s -check-prefix CHECK-EXECUTE-ONLY-NO-MOVT
 // CHECK-EXECUTE-ONLY-NO-MOVT: error: option '-mexecute-only' cannot be specified with '-mno-movt'
 
 // RUN: %clang -target armv7m-eabi -x assembler -mexecute-only %s -c -### 2>&1 \
 // RUN:    | FileCheck %s --check-prefix=CHECK-NO-EXECUTE-ONLY-ASM
 // CHECK-NO-EXECUTE-ONLY-ASM: warning: argument unused during compilation: '-mexecute-only'
 
 // -mpure-code flag for GCC compatibility
-// RUN: not %clang -c -target thumbv6m-eabi -mpure-code %s 2>&1 | \
+// RUN: not %clang -c -target armv6-eabi -mpure-code %s 2>&1 | \
 // RUN:   FileCheck --check-prefix CHECK-EXECUTE-ONLY-NOT-SUPPORTED %s
 
 // RUN: not %clang -target armv8m.main-eabi -mpure-code -mno-movt %s 2>&1 \
 // RUN:    | FileCheck %s -check-prefix CHECK-PURE-CODE-NO-MOVT
 // CHECK-PURE-CODE-NO-MOVT: error: option '-mpure-code' cannot be specified with '-mno-movt'

llvm/lib/Target/ARM/ARMSubtarget.cpp

else
       ArchFS = std::string(FS);
   }
   ParseSubtargetFeatures(CPUString, /*TuneCPU*/ CPUString, ArchFS);
 
   // FIXME: This used enable V6T2 support implicitly for Thumb2 mode.
   // Assert this for now to make the change obvious.
   assert(hasV6T2Ops() || !hasThumb2());
 
-  // Execute only support requires movt support
   if (genExecuteOnly()) {
+    // Execute only support for >= v8-M Baseline requires movt support
+    if (hasV8MBaselineOps())
       NoMovt = false;
-    assert(hasV8MBaselineOps() && "Cannot generate execute-only code for this target");
+    if (!hasV6MOps())
+      llvm_unreachable("Cannot generate execute-only code for this target");
   }
 
   // Keep a pointer to static instruction cost data for the specified CPU.
   SchedModel = getSchedModelForCPU(CPUString);
 
   // Initialize scheduling itinerary for the specified CPU.
   InstrItins = getInstrItineraryForCPU(CPUString);
 
bool ARMSubtarget::useStride4VFPs() const {
   return isTargetWatchABI() ||
          (useWideStrideVFP() && !OptMinSize);
 }
 
 bool ARMSubtarget::useMovt() const {
   // NOTE Windows on ARM needs to use mov.w/mov.t pairs to materialise 32-bit
   // immediates as it is inherently position independent, and may be out of
   // range otherwise.
   return !NoMovt && hasV8MBaselineOps() &&
          (isTargetWindows() || !OptMinSize || genExecuteOnly());
 }
 
 bool ARMSubtarget::useFastISel() const {
   // Enable fast-isel for any target, for testing only.
   if (ForceFastISel)
     return true;