[M68k] M68kExpandPseudo::ExpandMI and M68kInstrInfo::ExpandMOVEM incorrectly promote 8bit and 16bit registers to 32bit

[TechnoElf]

The functions M68kExpandPseudo::ExpandMI and M68kInstrInfo::ExpandMOVEM unconditionally promote MOVEM instructions to their 32bit variant to allow collapsing multiple consecutive instructions. This optimisation, however, clashes with the representation of these values in memory. For instance, a 16bit value will still only be allocated 2 bytes on the stack, causing unrelated data to be overwritten when the 32bit MOVEM is executed.

[TechnoElf]

I have solved this issue by removing the aforementioned register promotions, but I'm not sure if this is the correct approach:

--- a/llvm/lib/Target/M68k/M68kExpandPseudo.cpp
+++ b/llvm/lib/Target/M68k/M68kExpandPseudo.cpp
@@ -193,31 +193,23 @@ bool M68kExpandPseudo::ExpandMI(MachineBasicBlock &MBB,
   case M68k::MOV8dc:
     return TII->ExpandCCR(MIB, /*IsToCCR=*/false);

-  case M68k::MOVM8jm_P:
-    return TII->ExpandMOVEM(MIB, TII->get(M68k::MOVM32jm), /*IsRM=*/false);
   case M68k::MOVM16jm_P:
-    return TII->ExpandMOVEM(MIB, TII->get(M68k::MOVM32jm), /*IsRM=*/false);
+    return TII->ExpandMOVEM(MIB, TII->get(M68k::MOVM16jm), /*IsRM=*/false);
   case M68k::MOVM32jm_P:
     return TII->ExpandMOVEM(MIB, TII->get(M68k::MOVM32jm), /*IsRM=*/false);

-  case M68k::MOVM8pm_P:
-    return TII->ExpandMOVEM(MIB, TII->get(M68k::MOVM32pm), /*IsRM=*/false);
   case M68k::MOVM16pm_P:
-    return TII->ExpandMOVEM(MIB, TII->get(M68k::MOVM32pm), /*IsRM=*/false);
+    return TII->ExpandMOVEM(MIB, TII->get(M68k::MOVM16pm), /*IsRM=*/false);
   case M68k::MOVM32pm_P:
     return TII->ExpandMOVEM(MIB, TII->get(M68k::MOVM32pm), /*IsRM=*/false);

-  case M68k::MOVM8mj_P:
-    return TII->ExpandMOVEM(MIB, TII->get(M68k::MOVM32mj), /*IsRM=*/true);
   case M68k::MOVM16mj_P:
-    return TII->ExpandMOVEM(MIB, TII->get(M68k::MOVM32mj), /*IsRM=*/true);
+    return TII->ExpandMOVEM(MIB, TII->get(M68k::MOVM16mj), /*IsRM=*/true);
   case M68k::MOVM32mj_P:
     return TII->ExpandMOVEM(MIB, TII->get(M68k::MOVM32mj), /*IsRM=*/true);

-  case M68k::MOVM8mp_P:
-    return TII->ExpandMOVEM(MIB, TII->get(M68k::MOVM32mp), /*IsRM=*/true);
   case M68k::MOVM16mp_P:
-    return TII->ExpandMOVEM(MIB, TII->get(M68k::MOVM32mp), /*IsRM=*/true);
+    return TII->ExpandMOVEM(MIB, TII->get(M68k::MOVM16mp), /*IsRM=*/true);
   case M68k::MOVM32mp_P:
     return TII->ExpandMOVEM(MIB, TII->get(M68k::MOVM32mp), /*IsRM=*/true);

--- a/llvm/lib/Target/M68k/M68kInstrInfo.cpp
+++ b/llvm/lib/Target/M68k/M68kInstrInfo.cpp
@@ -559,10 +559,10 @@ bool M68kInstrInfo::ExpandMOVEM(MachineInstrBuilder &MIB,

   // If the register is not in XR32 then it is smaller than 32 bit, we
   // implicitly promote it to 32
-  if (!XR32->contains(Reg)) {
-    Reg = RI.getMatchingMegaReg(Reg, XR32);
-    assert(Reg && "Has not meaningful MEGA register");
-  }
+  //if (!XR32->contains(Reg)) {
+  //  Reg = RI.getMatchingMegaReg(Reg, XR32);
+  //  assert(Reg && "Has not meaningful MEGA register");
+  //}

   unsigned Mask = 1 << RI.getSpillRegisterOrder(Reg);
   if (IsRM) {

[llvmbot]

@llvm/issue-subscribers-backend-m68k

Author: Janis Heims (TechnoElf)

The functions `M68kExpandPseudo::ExpandMI` and `M68kInstrInfo::ExpandMOVEM` unconditionally promote MOVEM instructions to their 32bit variant to allow collapsing multiple consecutive instructions. This optimisation, however, clashes with the representation of these values in memory. For instance, a 16bit value will still only be allocated 2 bytes on the stack, causing unrelated data to be overwritten when the 32bit MOVEM is executed.