Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[M68k] Add support for bitwise NOT instruction #88049

Merged
merged 2 commits into from
Apr 9, 2024
Merged

Conversation

n8pjl
Copy link
Contributor

@n8pjl n8pjl commented Apr 8, 2024

Currently the bitwise NOT instruction is not recognized. Add support for
using NOT on data registers. This is a partial implementation that puts
NOT at the same level of support as NEG currently enjoys.

Using not rather than eori cuts the length of the encoded instruction
in half or in thirds, leading to a reduction of 4-10 cycles per
instruction, on the original 68000.

This change includes tests for both bitwise and arithmetic negation.

Currently the bitwise NOT instruction is not recognized. Add support for
using NOT on data registers. This is a partial implementation that puts
NOT at the same level of support as NEG currently enjoys.

Using not rather than eori cuts the length of the encoded instruction
in half or in thirds, leading to a reduction of 4-10 cycles per
instruction, on the original 68000.

This change includes tests for both bitwise and arithmetic negation.
Copy link

github-actions bot commented Apr 8, 2024

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 8, 2024

@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-m68k

Author: Peter Lafreniere (n8pjl)

Changes

Currently the bitwise NOT instruction is not recognized. Add support for
using NOT on data registers. This is a partial implementation that puts
NOT at the same level of support as NEG currently enjoys.

Using not rather than eori cuts the length of the encoded instruction
in half or in thirds, leading to a reduction of 4-10 cycles per
instruction, on the original 68000.

This change includes tests for both bitwise and arithmetic negation.


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

3 Files Affected:

  • (modified) llvm/lib/Target/M68k/M68kInstrArithmetic.td (+17-3)
  • (added) llvm/test/CodeGen/M68k/Arith/unary.ll (+86)
  • (modified) llvm/test/CodeGen/M68k/Atomics/rmw.ll (+1-1)
diff --git a/llvm/lib/Target/M68k/M68kInstrArithmetic.td b/llvm/lib/Target/M68k/M68kInstrArithmetic.td
index 3532e56e741705..e2d4e49ddf27b6 100644
--- a/llvm/lib/Target/M68k/M68kInstrArithmetic.td
+++ b/llvm/lib/Target/M68k/M68kInstrArithmetic.td
@@ -15,8 +15,8 @@
 ///    ADD       [~]   ADDA      [~]   ADDI        [~]   ADDQ [ ]   ADDX [~]
 ///    CLR       [ ]   CMP       [~]   CMPA        [~]   CMPI [~]   CMPM [ ]
 ///    CMP2      [ ]   DIVS/DIVU [~]   DIVSL/DIVUL [ ]   EXT  [~]   EXTB [ ]
-///    MULS/MULU [~]   NEG       [~]   NEGX        [~]   SUB  [~]   SUBA [~]
-///    SUBI      [~]   SUBQ      [ ]   SUBX        [~]
+///    MULS/MULU [~]   NEG       [~]   NEGX        [~]   NOT  [~]   SUB  [~]
+///    SUBA      [~]   SUBI      [~]   SUBQ        [ ]   SUBX [~]
 ///
 ///  Map:
 ///
@@ -769,7 +769,7 @@ def : Pat<(mulhu i16:$dst, Mxi16immSExt16:$opd),
 
 
 //===----------------------------------------------------------------------===//
-// NEG/NEGX
+// NEG/NEGX/NOT
 //===----------------------------------------------------------------------===//
 
 /// ------------+------------+------+---------+---------
@@ -809,12 +809,26 @@ class MxNegX_D<MxType TYPE>
 }
 }
 
+class MxNot_D<MxType TYPE>
+    : MxInst<(outs TYPE.ROp:$dst), (ins TYPE.ROp:$src),
+             "not."#TYPE.Prefix#"\t$dst",
+             [(set TYPE.VT:$dst, (not TYPE.VT:$src))]> {
+  let Inst = (descend 0b01000110,
+    /*SIZE*/!cast<MxEncSize>("MxEncSize"#TYPE.Size).Value,
+    //MODE without last bit
+    0b00,
+    //REGISTER prefixed by D/A bit
+    (operand "$dst", 4)
+  );
+}
+
 } // let Constraints
 } // let Defs = [CCR]
 
 foreach S = [8, 16, 32] in {
   def NEG#S#d  : MxNeg_D<!cast<MxType>("MxType"#S#"d")>;
   def NEGX#S#d : MxNegX_D<!cast<MxType>("MxType"#S#"d")>;
+  def NOT#S#d  : MxNot_D<!cast<MxType>("MxType"#S#"d")>;
 }
 
 def : Pat<(MxSub 0, i8 :$src), (NEG8d  MxDRD8 :$src)>;
diff --git a/llvm/test/CodeGen/M68k/Arith/unary.ll b/llvm/test/CodeGen/M68k/Arith/unary.ll
new file mode 100644
index 00000000000000..a28ac7328d2601
--- /dev/null
+++ b/llvm/test/CodeGen/M68k/Arith/unary.ll
@@ -0,0 +1,86 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --filter-out ";*\.cfi_*"
+; RUN: llc < %s -mtriple=m68k-linux -verify-machineinstrs | FileCheck %s
+
+define i64 @notll(i64 %x) {
+; CHECK-LABEL: notll:
+; CHECK:  ; %bb.0:
+; CHECK:    move.l (4,%sp), %d0
+; CHECK:    not.l %d0
+; CHECK:    move.l (8,%sp), %d1
+; CHECK:    not.l %d1
+; CHECK:    rts
+  %not = xor i64 %x, -1
+  ret i64 %not
+}
+
+define i32 @notl(i32 %x) {
+; CHECK-LABEL: notl:
+; CHECK:  ; %bb.0:
+; CHECK:    move.l (4,%sp), %d0
+; CHECK:    not.l %d0
+; CHECK:    rts
+  %not = xor i32 %x, -1
+  ret i32 %not
+}
+
+define i16 @nots(i16 %x) {
+; CHECK-LABEL: nots:
+; CHECK:  ; %bb.0:
+; CHECK:    move.w (6,%sp), %d0
+; CHECK:    not.w %d0
+; CHECK:    rts
+  %not = xor i16 %x, -1
+  ret i16 %not
+}
+
+define i8 @notb(i8 %x) {
+; CHECK-LABEL: notb:
+; CHECK:  ; %bb.0:
+; CHECK:    move.b (7,%sp), %d0
+; CHECK:    not.b %d0
+; CHECK:    rts
+  %not = xor i8 %x, -1
+  ret i8 %not
+}
+
+define i64 @negll(i64 %x) {
+; CHECK-LABEL: negll:
+; CHECK:  ; %bb.0:
+; CHECK:    move.l (4,%sp), %d0
+; CHECK:    move.l (8,%sp), %d1
+; CHECK:    neg.l %d1
+; CHECK:    negx.l %d0
+; CHECK:    rts
+  %neg = sub i64 0, %x
+  ret i64 %neg
+}
+
+define i32 @negl(i32 %x) {
+; CHECK-LABEL: negl:
+; CHECK:  ; %bb.0:
+; CHECK:    move.l (4,%sp), %d0
+; CHECK:    neg.l %d0
+; CHECK:    rts
+  %neg = sub i32 0, %x
+  ret i32 %neg
+}
+
+define i16 @negs(i16 %x) {
+; CHECK-LABEL: negs:
+; CHECK:  ; %bb.0:
+; CHECK:    move.w (6,%sp), %d0
+; CHECK:    neg.w %d0
+; CHECK:    rts
+  %neg = sub i16 0, %x
+  ret i16 %neg
+}
+
+define i8 @negb(i8 %x) {
+; CHECK-LABEL: negb:
+; CHECK:  ; %bb.0:
+; CHECK:    move.b (7,%sp), %d0
+; CHECK:    neg.b %d0
+; CHECK:    rts
+  %neg = sub i8 0, %x
+  ret i8 %neg
+}
diff --git a/llvm/test/CodeGen/M68k/Atomics/rmw.ll b/llvm/test/CodeGen/M68k/Atomics/rmw.ll
index b589e7751d80e7..1036a0a8ba3d25 100644
--- a/llvm/test/CodeGen/M68k/Atomics/rmw.ll
+++ b/llvm/test/CodeGen/M68k/Atomics/rmw.ll
@@ -237,7 +237,7 @@ define i16 @atmoicrmw_nand_i16(i16 %val, ptr %ptr) {
 ; ATOMIC-NEXT:    ; =>This Inner Loop Header: Depth=1
 ; ATOMIC-NEXT:    move.w %d2, %d3
 ; ATOMIC-NEXT:    and.w %d0, %d3
-; ATOMIC-NEXT:    eori.w #-1, %d3
+; ATOMIC-NEXT:    not.w %d3
 ; ATOMIC-NEXT:    cas.w %d1, %d3, (%a0)
 ; ATOMIC-NEXT:    move.w %d1, %d3
 ; ATOMIC-NEXT:    sub.w %d2, %d3

@mshockwave mshockwave self-requested a review April 8, 2024 21:39
@mshockwave
Copy link
Member

Thank you for the patch. Overall looks pretty good, you did a great job on your first LLVM PR!
Could you also add MC (a.k.a assembler & disassembler) tests as well? You can take a look at other MC tests under llvm/test/MC/M68k and llvm/test/MC/Disassembler/M68k.

@n8pjl
Copy link
Contributor Author

n8pjl commented Apr 8, 2024

Thank you, and yes, I'll write a few tests for MC. I didn't know that those existed.
I did make sure that inline assembly works, but only for data register direct (it's the same with neg[x]).

Does LLVM have a preference over if the MC tests go into a second commit, or if I revise the first commit and force push?

@mshockwave
Copy link
Member

Does LLVM have a preference over if the MC tests go into a second commit, or if I revise the first commit and force push?

The current policy states that during code review, all changes/fixes to the PR should be put in separate commits (unless there is a substantial change on the main branch that will break the PR and you have to rebase + forced push) so the reviewer can see a clear history of changes. But once the code reviewer is done, you have to squash all the commits before merge. Actually, I think we make "Squash and merge" button the only clickable one on the web UI.

@n8pjl
Copy link
Contributor Author

n8pjl commented Apr 8, 2024

Good to know, I like that policy.

I'm used to email-based patch management, so using PRs takes some getting used to.

@llvmbot llvmbot added the mc Machine (object) code label Apr 8, 2024
Copy link
Member

@mshockwave mshockwave left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Please let me know if you're not able to merge the PR (I'm not really sure how they setup permission for new contributors after we migrate to GitHub PR). If you need help to merge, please make sure the email you want to use in the commit is in your account's public email list, otherwise GitHub will assign an ugly "XXX-noreply@github.com" to the commit.

@n8pjl
Copy link
Contributor Author

n8pjl commented Apr 9, 2024

I'm not allowed to merge the PR so it would be great if you could do that for me.

@mshockwave mshockwave merged commit 614a578 into llvm:main Apr 9, 2024
4 checks passed
Copy link

github-actions bot commented Apr 9, 2024

@n8pjl Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested
by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself.
This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

@n8pjl n8pjl deleted the m68k-not branch April 9, 2024 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:m68k mc Machine (object) code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants