[ARM] Avoid switching ARM/Thumb mode on .arch/.cpu directive

[olista01]

Summary

When we see a .arch or .cpu directive, we should try to avoid switching
ARM/Thumb mode if possible.

If we do have to switch modes, we also need to emit the correct mapping
symbol for the new ISA. We did not do this previously, so could emit
ARM code with Thumb mapping symbols (or vice-versa).

The GAS behaviour is to always stay in the same mode, and to emit an
error on any instructions seen when the current mode is not available on
the current target. We can't represent that situation easily (we assume
that Thumb mode is available if ModeThumb is set), so we differ from the
GAS behaviour when switching to a target that can't support the old
mode. I've added a warning for when this implicit mode-switch occurs.

[rengolin]

Hum, this is an interesting one, and is probably only relevant for v6M and v7M.

I don't like deviating from what GNU tools do, but in this case, I don't see an easy solution.

The problem in doing this is that Clang users will have trouble using GAS, since they won't need the .code 16 for Clang, but will for GAS. The fix is easy, though, just add another .code 16 and job's done, so I agree this is not a big deal.

GCC already adds .code directives on inline assembly, and that has caused us some grief, but again, nothing serious, just a bit ugly.

Since this is no-man's land, and the change is mostly harmless, I'm ok with it (with the two extra tests).

LGTM. Thanks!

test/MC/ARM/directive-arch-mode-switch.s

25 ↗

(On Diff #53217)

Add two .cpu tests, for coverage. Ex. v7 -> M0 -> A9

llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp

bool hasV8_1aOps() const {
     return getSTI().getFeatureBits()[ARM::HasV8_1aOps];
   }
 
   void SwitchMode() {
     MCSubtargetInfo &STI = copySTI();
     uint64_t FB = ComputeAvailableFeatures(STI.ToggleFeature(ARM::ModeThumb));
     setAvailableFeatures(FB);
   }
+  void FixModeAfterArchChange(bool WasThumb, SMLoc Loc);
   bool isMClass() const {
     return getSTI().getFeatureBits()[ARM::FeatureMClass];
   }
 
   /// @name Auto-generated Match Functions
   /// {
 
 #define GET_ASSEMBLER_HEADER
if (Parser.getTok().isNot(AsmToken::Identifier)) {
     Error(L, "unexpected input in .unreq directive.");
     return false;
   }
   RegisterReqs.erase(Parser.getTok().getIdentifier().lower());
   Parser.Lex(); // Eat the identifier.
   return false;
 }
 
+// After changing arch/CPU, try to put the ARM/Thumb mode back to what it was
+// before, if supported by the new target, or emit mapping symbols for the mode
+// switch.
+void ARMAsmParser::FixModeAfterArchChange(bool WasThumb, SMLoc Loc) {
+  if (WasThumb != isThumb()) {
+    if (WasThumb && hasThumb()) {
+      // Stay in Thumb mode
+      SwitchMode();
+    } else if (!WasThumb && hasARM()) {
+      // Stay in ARM mode
+      SwitchMode();
+    } else {
+      // Mode switch forced, because the new arch doesn't support the old mode.
+      getParser().getStreamer().EmitAssemblerFlag(isThumb() ? MCAF_Code16
+                                                            : MCAF_Code32);
+      // Warn about the implcit mode switch. GAS does not switch modes here,
+      // but instead stays in the old mode, reporting an error on any following
+      // instructions as the mode does not exist on the target.
+      Warning(Loc, Twine("new target does not support ") +
+                       (WasThumb ? "thumb" : "arm") + " mode, switching to " +
+                       (!WasThumb ? "thumb" : "arm") + " mode");
+    }
+  }
+}
+
 /// parseDirectiveArch
 ///  ::= .arch token
 bool ARMAsmParser::parseDirectiveArch(SMLoc L) {
   StringRef Arch = getParser().parseStringToEndOfStatement().trim();
 
   unsigned ID = ARM::parseArch(Arch);
 
   if (ID == ARM::AK_INVALID) {
     Error(L, "Unknown arch name");
     return false;
   }
 
+  bool WasThumb = isThumb();
   Triple T;
   MCSubtargetInfo &STI = copySTI();
   STI.setDefaultFeatures("", ("+" + ARM::getArchName(ID)).str());
   setAvailableFeatures(ComputeAvailableFeatures(STI.getFeatureBits()));
+  FixModeAfterArchChange(WasThumb, L);
 
   getTargetStreamer().emitArch(ID);
   return false;
 }
 
 /// parseDirectiveEabiAttr
 ///  ::= .eabi_attribute int, int [, "str"]
 ///  ::= .eabi_attribute Tag_name, int [, "str"]
bool ARMAsmParser::parseDirectiveCPU(SMLoc L) {
 
   // FIXME: This is using table-gen data, but should be moved to
   // ARMTargetParser once that is table-gen'd.
   if (!getSTI().isCPUStringValid(CPU)) {
     Error(L, "Unknown CPU name");
     return false;
   }
 
+  bool WasThumb = isThumb();
   MCSubtargetInfo &STI = copySTI();
   STI.setDefaultFeatures(CPU, "");
   setAvailableFeatures(ComputeAvailableFeatures(STI.getFeatureBits()));
+  FixModeAfterArchChange(WasThumb, L);
 
   return false;
 }
 /// parseDirectiveFPU
 ///  ::= .fpu str
 bool ARMAsmParser::parseDirectiveFPU(SMLoc L) {
   SMLoc FPUNameLoc = getTok().getLoc();
   StringRef FPU = getParser().parseStringToEndOfStatement().trim();

llvm/trunk/test/MC/ARM/directive-arch-mode-switch.s

+@ RUN: llvm-mc -triple arm-none-eabi -filetype asm %s 2>%t | FileCheck %s
+@ RUN: FileCheck %s <%t --check-prefix=STDERR
+
+@ Start in arm mode
+  .arm
+@ CHECK: .code 32
+
+@ In ARM mode, switch to an arch which has ARM and Thumb, no warning or .code directive (stay in ARM mode)
+  .arch armv7-a
+@ STDERR-NOT: [[@LINE-1]]:{{[0-9]+}}: warning:
+@ CHECK-NOT: .code
+@ CHECK: .arch   armv7-a
+@ CHECK-NOT: .code
+
+@ In ARM mode, switch to an arch which has Thumb only, expect warning and .code 16 directive
+  .arch armv6-m
+@ STDERR: [[@LINE-1]]:{{[0-9]+}}: warning: new target does not support arm mode, switching to thumb mode
+@ CHECK: .code   16
+@ CHECK: .arch   armv6-m
+
+@ In Thumb mode, switch to an arch which has ARM and Thumb, no warning or .code directive (stay in Thumb mode)
+  .arch armv7-a
+@ STDERR-NOT: [[@LINE-1]]:{{[0-9]+}}: warning:
+@ CHECK-NOT: .code
+@ CHECK: .arch   armv7-a
+@ CHECK-NOT: .code
+
+@ In Thumb mode, switch to a CPU which has ARM and Thumb, no warning or .code directive (stay in Thumb mode)
+  .cpu cortex-a8
+@ STDERR-NOT: [[@LINE-1]]:{{[0-9]+}}: warning:
+@ CHECK-NOT: .code
+@ CHECK: .cpu cortex-a8
+@ CHECK-NOT: .code
+
+@ Switch to ARM mode
+  .arm
+@ CHECK: .code 32
+
+@ In ARM mode, switch to a CPU which has ARM and Thumb, no warning or .code directive (stay in ARM mode)
+  .cpu cortex-a8
+@ STDERR-NOT: [[@LINE-1]]:{{[0-9]+}}: warning:
+@ CHECK-NOT: .code
+@ CHECK: .cpu cortex-a8
+@ CHECK-NOT: .code
+
+@ In ARM mode, switch to a CPU which has Thumb only, expect warning and .code 16 directive
+  .cpu cortex-m3
+@ STDERR: [[@LINE-1]]:{{[0-9]+}}: warning: new target does not support arm mode, switching to thumb mode
+@ CHECK: .cpu    cortex-m3
+@ CHECK: .code   16
+
+@ We don't have any ARM-only targets (i.e. v4), so we can't test the forced Thumb->ARM case