[M68k] Handle 16 bit MOVs to and from CCR #114714

knickish · 2024-11-03T17:53:02Z

Builds on @TechnoElf 's CCR MOV pr #107591 and adds some tests.

Fixes #106210.

llvmbot · 2024-11-03T18:49:09Z

@llvm/pr-subscribers-backend-m68k

Author: None (knickish)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/114714.diff

4 Files Affected:

(modified) llvm/lib/Target/M68k/M68kInstrData.td (+3-1)
(modified) llvm/lib/Target/M68k/M68kInstrInfo.cpp (+33-11)
(modified) llvm/lib/Target/M68k/M68kRegisterInfo.h (+1-1)
(added) llvm/test/CodeGen/M68k/Control/non-cmov-switch.ll (+126)

diff --git a/llvm/lib/Target/M68k/M68kInstrData.td b/llvm/lib/Target/M68k/M68kInstrData.td
index dc777a933e2786..adaa8c85eefb9c 100644
--- a/llvm/lib/Target/M68k/M68kInstrData.td
+++ b/llvm/lib/Target/M68k/M68kInstrData.td
@@ -419,6 +419,8 @@ class MxMoveFromCCR_M<MxOperand MEMOp, MxEncMemOp DST_ENC>
 
 class MxMoveFromCCRPseudo<MxOperand MEMOp>
     : MxPseudo<(outs), (ins MEMOp:$dst, CCRC:$src)>;
+class MxMoveFromCCR_RPseudo<MxOperand MEMOp>
+    : MxPseudo<(outs MEMOp:$dst), (ins CCRC:$src)>;
 } // let Uses = [CCR]
 
 let mayStore = 1 in
@@ -432,7 +434,7 @@ foreach AM = MxMoveSupportedAMs in {
 
 // Only data register is allowed.
 def MOV16dc : MxMoveFromCCR_R;
-def MOV8dc  : MxMoveFromCCRPseudo<MxOp8AddrMode_d.Op>;
+def MOV8dc  : MxMoveFromCCR_RPseudo<MxOp8AddrMode_d.Op>;
 
 //===----------------------------------------------------------------------===//
 // LEA
diff --git a/llvm/lib/Target/M68k/M68kInstrInfo.cpp b/llvm/lib/Target/M68k/M68kInstrInfo.cpp
index 2d9285f89b74ae..25b7b0bdb2a0c5 100644
--- a/llvm/lib/Target/M68k/M68kInstrInfo.cpp
+++ b/llvm/lib/Target/M68k/M68kInstrInfo.cpp
@@ -23,7 +23,9 @@
 #include "llvm/CodeGen/LivePhysRegs.h"
 #include "llvm/CodeGen/LiveVariables.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
+#include "llvm/CodeGen/MachineOperand.h"
 #include "llvm/CodeGen/MachineRegisterInfo.h"
+#include "llvm/CodeGenTypes/MachineValueType.h"
 #include "llvm/MC/TargetRegistry.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/Regex.h"
@@ -571,6 +573,17 @@ bool M68kInstrInfo::ExpandPUSH_POP(MachineInstrBuilder &MIB,
 }
 
 bool M68kInstrInfo::ExpandCCR(MachineInstrBuilder &MIB, bool IsToCCR) const {
+  switch (MIB->getOpcode()) {
+  case M68k::MOV8cd: {
+    // Promote used register to the next class
+    auto &Opd = MIB->getOperand(1);
+    Opd.setReg(getRegisterInfo().getMatchingSuperReg(
+        Opd.getReg(), M68k::MxSubRegIndex8Lo, &M68k::DR16RegClass));
+    break;
+  }
+  default:
+    break;
+  }
 
   // Replace the pseudo instruction with the real one
   if (IsToCCR)
@@ -579,11 +592,6 @@ bool M68kInstrInfo::ExpandCCR(MachineInstrBuilder &MIB, bool IsToCCR) const {
     // FIXME M68010 or later is required
     MIB->setDesc(get(M68k::MOV16dc));
 
-  // Promote used register to the next class
-  auto &Opd = MIB->getOperand(1);
-  Opd.setReg(getRegisterInfo().getMatchingSuperReg(
-      Opd.getReg(), M68k::MxSubRegIndex8Lo, &M68k::DR16RegClass));
-
   return true;
 }
 
@@ -757,13 +765,27 @@ void M68kInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
   bool ToSR = DstReg == M68k::SR;
 
   if (FromCCR) {
-    assert(M68k::DR8RegClass.contains(DstReg) &&
-           "Need DR8 register to copy CCR");
-    Opc = M68k::MOV8dc;
+    if (M68k::DR8RegClass.contains(DstReg)) {
+      Opc = M68k::MOV8dc;
+    } else if (M68k::DR16RegClass.contains(DstReg)) {
+      Opc = M68k::MOV16dc;
+    } else if (M68k::DR32RegClass.contains(DstReg)) {
+      Opc = M68k::MOV16dc;
+    } else {
+      LLVM_DEBUG(dbgs() << "Cannot copy CCR to " << RI.getName(DstReg) << '\n');
+      llvm_unreachable("Invalid register for MOVE from CCR");
+    }
   } else if (ToCCR) {
-    assert(M68k::DR8RegClass.contains(SrcReg) &&
-           "Need DR8 register to copy CCR");
-    Opc = M68k::MOV8cd;
+    if (M68k::DR8RegClass.contains(SrcReg)) {
+      Opc = M68k::MOV8cd;
+    } else if (M68k::DR16RegClass.contains(SrcReg)) {
+      Opc = M68k::MOV16cd;
+    } else if (M68k::DR32RegClass.contains(SrcReg)) {
+      Opc = M68k::MOV16cd;
+    } else {
+      LLVM_DEBUG(dbgs() << "Cannot copy " << RI.getName(SrcReg) << " to CCR\n");
+      llvm_unreachable("Invalid register for MOVE to CCR");
+    }
   } else if (FromSR || ToSR)
     llvm_unreachable("Cannot emit SR copy instruction");
 
diff --git a/llvm/lib/Target/M68k/M68kRegisterInfo.h b/llvm/lib/Target/M68k/M68kRegisterInfo.h
index 0cfab150d0c84b..51317683453666 100644
--- a/llvm/lib/Target/M68k/M68kRegisterInfo.h
+++ b/llvm/lib/Target/M68k/M68kRegisterInfo.h
@@ -101,7 +101,7 @@ class M68kRegisterInfo : public M68kGenRegisterInfo {
   const TargetRegisterClass *
   getCrossCopyRegClass(const TargetRegisterClass *RC) const override {
     if (RC == &M68k::CCRCRegClass)
-      return &M68k::DR32RegClass;
+      return &M68k::DR16RegClass;
     return RC;
   }
 
diff --git a/llvm/test/CodeGen/M68k/Control/non-cmov-switch.ll b/llvm/test/CodeGen/M68k/Control/non-cmov-switch.ll
new file mode 100644
index 00000000000000..90d2be017ecdb2
--- /dev/null
+++ b/llvm/test/CodeGen/M68k/Control/non-cmov-switch.ll
@@ -0,0 +1,126 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc < %s -mtriple=m68k-linux  -mcpu=M68020 --verify-machineinstrs | FileCheck %s
+
+define internal void @select_i32(i32 %self, ptr nonnull %value) {
+; CHECK-LABEL: select_i32:
+; CHECK:         .cfi_startproc
+; CHECK-NEXT:  ; %bb.0: ; %start
+; CHECK-NEXT:    suba.l #4, %sp
+; CHECK-NEXT:    .cfi_def_cfa_offset -8
+; CHECK-NEXT:    movem.l %d2, (0,%sp) ; 8-byte Folded Spill
+; CHECK-NEXT:    cmpi.l #0, (8,%sp)
+; CHECK-NEXT:    move.w %ccr, %d2
+; CHECK-NEXT:    sne %d1
+; CHECK-NEXT:    move.l (12,%sp), %d0
+; CHECK-NEXT:    move.w %d2, %ccr
+; CHECK-NEXT:    bne .LBB0_2
+; CHECK-NEXT:  ; %bb.1: ; %start
+; CHECK-NEXT:    and.l #255, %d1
+; CHECK-NEXT:    cmpi.l #0, %d1
+; CHECK-NEXT:    bne .LBB0_3
+; CHECK-NEXT:  .LBB0_2: ; %null
+; CHECK-NEXT:    suba.l %a0, %a0
+; CHECK-NEXT:    move.l %d0, (%a0)
+; CHECK-NEXT:  .LBB0_3: ; %exit
+; CHECK-NEXT:    movem.l (0,%sp), %d2 ; 8-byte Folded Reload
+; CHECK-NEXT:    adda.l #4, %sp
+; CHECK-NEXT:    rts
+start:
+  %2 = icmp eq i32 %self, 0
+  %3 = select i1 %2, i32 0, i32 1
+  switch i32 %3, label %exit [
+    i32 0, label %nonnull
+    i32 1, label %null
+  ]
+
+nonnull:                                              ; preds = %start
+  store ptr %value, ptr null, align 2
+  br label %exit
+
+null:                                              ; preds = %start
+  store ptr %value, ptr null, align 2
+  br label %exit
+
+exit:                                              ; preds = %nonnull, %null
+  ret void
+}
+
+define internal void @select_i16(i16 %self, ptr nonnull %value) {
+; CHECK-LABEL: select_i16:
+; CHECK:         .cfi_startproc
+; CHECK-NEXT:  ; %bb.0: ; %start
+; CHECK-NEXT:    suba.l #4, %sp
+; CHECK-NEXT:    .cfi_def_cfa_offset -8
+; CHECK-NEXT:    movem.l %d2, (0,%sp) ; 8-byte Folded Spill
+; CHECK-NEXT:    cmpi.w #0, (10,%sp)
+; CHECK-NEXT:    move.w %ccr, %d2
+; CHECK-NEXT:    sne %d1
+; CHECK-NEXT:    move.l (12,%sp), %d0
+; CHECK-NEXT:    move.w %d2, %ccr
+; CHECK-NEXT:    bne .LBB1_2
+; CHECK-NEXT:  ; %bb.1: ; %start
+; CHECK-NEXT:    and.l #255, %d1
+; CHECK-NEXT:    cmpi.w #0, %d1
+; CHECK-NEXT:    bne .LBB1_3
+; CHECK-NEXT:  .LBB1_2: ; %null
+; CHECK-NEXT:    suba.l %a0, %a0
+; CHECK-NEXT:    move.l %d0, (%a0)
+; CHECK-NEXT:  .LBB1_3: ; %exit
+; CHECK-NEXT:    movem.l (0,%sp), %d2 ; 8-byte Folded Reload
+; CHECK-NEXT:    adda.l #4, %sp
+; CHECK-NEXT:    rts
+start:
+  %2 = icmp eq i16 %self, 0
+  %3 = select i1 %2, i16 0, i16 1
+  switch i16 %3, label %exit [
+    i16 0, label %nonnull
+    i16 1, label %null
+  ]
+
+nonnull:                                              ; preds = %start
+  store ptr %value, ptr null, align 2
+  br label %exit
+
+null:                                              ; preds = %start
+  store ptr %value, ptr null, align 2
+  br label %exit
+
+exit:                                              ; preds = %nonnull, %null
+  ret void
+}
+
+define internal void @select_i8(i8 %self, ptr nonnull %value) {
+; CHECK-LABEL: select_i8:
+; CHECK:         .cfi_startproc
+; CHECK-NEXT:  ; %bb.0: ; %start
+; CHECK-NEXT:    move.l (8,%sp), %d0
+; CHECK-NEXT:    cmpi.b #0, (7,%sp)
+; CHECK-NEXT:    sne %d1
+; CHECK-NEXT:    bne .LBB2_2
+; CHECK-NEXT:  ; %bb.1: ; %start
+; CHECK-NEXT:    cmpi.b #0, %d1
+; CHECK-NEXT:    bne .LBB2_3
+; CHECK-NEXT:  .LBB2_2: ; %null
+; CHECK-NEXT:    suba.l %a0, %a0
+; CHECK-NEXT:    move.l %d0, (%a0)
+; CHECK-NEXT:  .LBB2_3: ; %exit
+; CHECK-NEXT:    rts
+start:
+  %2 = icmp eq i8 %self, 0
+  %3 = select i1 %2, i8 0, i8 1
+  switch i8 %3, label %exit [
+    i8 0, label %nonnull
+    i8 1, label %null
+  ]
+
+nonnull:                                              ; preds = %start
+  store ptr %value, ptr null, align 2
+  br label %exit
+
+null:                                              ; preds = %start
+  store ptr %value, ptr null, align 2
+  br label %exit
+
+exit:                                              ; preds = %nonnull, %null
+  ret void
+}

glaubitz · 2024-11-05T07:45:21Z

CC @mshockwave @0x59616e

glaubitz · 2024-11-05T07:46:45Z

Thanks a lot for the contribution!

Could you start the subject with "[M68k]" and use a more descriptive title.

I'll have a go at testing your changes later today.

knickish · 2024-11-05T16:28:52Z

@glaubitz Thanks, much appreciated. I updated the title as requested.

How are you testing these? I set up a docker container chroot thing based on your guide on the debian forums, but it's pretty unwieldy to be honest

glaubitz · 2024-11-05T19:37:06Z

@glaubitz Thanks, much appreciated. I updated the title as requested.

How are you testing these? I set up a docker container chroot thing based on your guide on the debian forums, but it's pretty unwieldy to be honest

I'm currently doing cross-builds only as LLVM doesn't work natively on M68k at the moment due to the natural 2-byte alignment of the current ABI.

We're working on switching the ABI to 4-byte alignment by default, but there has been some resistance from a few people who insist on ABI compatibility.

However, both Debian and Gentoo agree that the port will only be maintainable in the future with 4-byte alignment by default.

Having said that, are there any specific issues you need help with?

knickish · 2024-11-06T06:03:46Z

I was just wondering what tests I should be running to verify that I'm not breaking anything, as the ones I have are super basic. The clang test suite maybe?

glaubitz · 2024-11-06T06:12:03Z

I was just wondering what tests I should be running to verify that I'm not breaking anything, as the ones I have are super basic. The clang test suite maybe?

Yes, running the testsuite is fine. FWIW, we have a buildbot for LLVM M68k to run integration tests.

mshockwave · 2024-11-10T23:36:57Z

llvm/lib/Target/M68k/M68kInstrInfo.cpp

+  switch (MIB->getOpcode()) {
+  case M68k::MOV8cd: {
+    // Promote used register to the next class
+    auto &Opd = MIB->getOperand(1);


please spell out the type here.

mshockwave · 2024-11-10T23:38:19Z

llvm/lib/Target/M68k/M68kInstrInfo.cpp

@@ -571,6 +573,17 @@ bool M68kInstrInfo::ExpandPUSH_POP(MachineInstrBuilder &MIB,
 }

 bool M68kInstrInfo::ExpandCCR(MachineInstrBuilder &MIB, bool IsToCCR) const {
+  switch (MIB->getOpcode()) {


could we just use if statement here?

switched to an if statement

mshockwave · 2024-11-10T23:40:03Z

I was just wondering what tests I should be running to verify that I'm not breaking anything, as the ones I have are super basic. The clang test suite maybe?

Usually LLVM's regression tests should be sufficient:

$ ninja check-llvm-codegen-m68k

llvm/lib/Target/M68k/M68kInstrInfo.cpp

knickish · 2024-11-22T20:44:46Z

Ping @mshockwave, all PR comments have been addressed

mshockwave

LGTM thanks!

knickish · 2024-11-27T17:03:01Z

Would someone be able to merge this? I don't have the ability to do that

[M68k] allow 16-bit registers for MOVE to/from CCR

0f92de6

knickish force-pushed the m68k-ccr-mov-w-tests branch 3 times, most recently from 8bdc22f to bc80cba Compare November 3, 2024 18:48

knickish marked this pull request as ready for review November 3, 2024 18:48

llvmbot added the backend:m68k label Nov 3, 2024

TechnoElf mentioned this pull request Nov 4, 2024

[M68k] allow 16-bit registers for MOVE to/from CCR #107591

Closed

workingjubilee mentioned this pull request Nov 5, 2024

m68k-unknown-linux-gnu: can't compile functions with certain return types rust-lang/rust#89498

Open

nikic requested a review from mshockwave November 5, 2024 09:22

knickish changed the title ~~M68k ccr mov~~ [M68k] Handle 16 bit MOVs to and from CCR Nov 5, 2024

glaubitz mentioned this pull request Nov 9, 2024

[M68k] Add remaining addressing modes for Atomic operations #115523

Open

mshockwave reviewed Nov 10, 2024

View reviewed changes

knickish force-pushed the m68k-ccr-mov-w-tests branch from bc80cba to 5fc6665 Compare November 11, 2024 05:01

knickish commented Nov 11, 2024

View reviewed changes

llvm/lib/Target/M68k/M68kInstrInfo.cpp Outdated Show resolved Hide resolved

[M68k] add test and change default copy size for ccr

025b5a1

knickish force-pushed the m68k-ccr-mov-w-tests branch from 5fc6665 to 025b5a1 Compare November 11, 2024 05:34

knickish requested a review from mshockwave November 17, 2024 18:03

mshockwave approved these changes Nov 22, 2024

View reviewed changes

mshockwave merged commit c29e895 into llvm:main Nov 27, 2024
6 of 8 checks passed

knickish mentioned this pull request Dec 8, 2024

Request Commit Access For knickish #119142

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[M68k] Handle 16 bit MOVs to and from CCR #114714

[M68k] Handle 16 bit MOVs to and from CCR #114714

knickish commented Nov 3, 2024 •

edited by nikic

Loading

llvmbot commented Nov 3, 2024

glaubitz commented Nov 5, 2024

glaubitz commented Nov 5, 2024

knickish commented Nov 5, 2024

glaubitz commented Nov 5, 2024

knickish commented Nov 6, 2024

glaubitz commented Nov 6, 2024

mshockwave Nov 10, 2024

knickish Nov 11, 2024

mshockwave Nov 10, 2024

knickish Nov 11, 2024

mshockwave commented Nov 10, 2024

knickish commented Nov 22, 2024

mshockwave left a comment

knickish commented Nov 27, 2024

[M68k] Handle 16 bit MOVs to and from CCR #114714

[M68k] Handle 16 bit MOVs to and from CCR #114714

Conversation

knickish commented Nov 3, 2024 • edited by nikic Loading

llvmbot commented Nov 3, 2024

glaubitz commented Nov 5, 2024

glaubitz commented Nov 5, 2024

knickish commented Nov 5, 2024

glaubitz commented Nov 5, 2024

knickish commented Nov 6, 2024

glaubitz commented Nov 6, 2024

mshockwave Nov 10, 2024

Choose a reason for hiding this comment

knickish Nov 11, 2024

Choose a reason for hiding this comment

mshockwave Nov 10, 2024

Choose a reason for hiding this comment

knickish Nov 11, 2024

Choose a reason for hiding this comment

mshockwave commented Nov 10, 2024

knickish commented Nov 22, 2024

mshockwave left a comment

Choose a reason for hiding this comment

knickish commented Nov 27, 2024

knickish commented Nov 3, 2024 •

edited by nikic

Loading