Page MenuHomePhabricator

[cfe][M68k] (Patch 7/8) Basic Clang support
AcceptedPublic

Authored by myhsu on Sep 27 2020, 10:27 PM.

Details

Summary
  1. Add M68k as new Clang target
  2. Add new attribute to support M68k's ISR (Interrupt Service Routine)

Diff Detail

Event Timeline

myhsu created this revision.Sep 27 2020, 10:27 PM
Herald added a project: Restricted Project. · View Herald Transcript
myhsu requested review of this revision.Sep 27 2020, 10:27 PM
myhsu updated this revision to Diff 295468.Sep 30 2020, 10:15 PM

Fix formatting issues

myhsu updated this revision to Diff 295706.Oct 1 2020, 5:24 PM

Update licenses

myhsu updated this revision to Diff 296064.EditedOct 4 2020, 11:38 AM

Fix the CPU name passing to the backend. Also removed M68060 for now, since backend hasn't fully implemented it yet.

myhsu updated this revision to Diff 302187.Nov 1 2020, 4:31 PM
myhsu retitled this revision from [cfe][M68K] (Patch 7/8) Basic Clang support to [cfe][M68k] (Patch 7/8) Basic Clang support.
myhsu edited the summary of this revision. (Show Details)

[NFC] Rename M680x0 to M68k

rengolin added inline comments.Nov 17 2020, 3:31 AM
clang/lib/Basic/Targets.cpp
314

No support for bare-metal?

clang/lib/Basic/Targets/M68k.cpp
123

no "sp", "pc", etc? Or are all of them aliased to those?

clang/lib/CodeGen/TargetInfo.cpp
8083

That's an odd comment... :)

What doesn't work right now? Something or everything?

RKSimon added inline comments.Nov 17 2020, 3:52 AM
clang/lib/Basic/CMakeLists.txt
78

(sorting) - move after Le64.cpp

clang/lib/CodeGen/TargetInfo.cpp
8087

(style) Use const auto * for cast results

bruno added a subscriber: bruno.Nov 19 2020, 11:37 AM

Looking forward to see m68k support (and hopefully sega genesis toolchain support someday)!

clang/lib/Basic/Targets/M68k.cpp
73

This doesn't seem to match the coding style, clang-format to the rescue.

89

Can you make all the defineMacro inline within the cases? Clarity seems more useful here.

clang/lib/Basic/Targets/M68k.h
35

Can you remove the comment and add the enum back whenever you introduce CK_68060 support?

myhsu updated this revision to Diff 308550.Nov 30 2020, 11:12 PM
myhsu marked 5 inline comments as done.
  • Rebased to latest changes
  • Addressed some of the feedbacks
myhsu updated this revision to Diff 308866.Dec 1 2020, 9:44 PM
myhsu marked 3 inline comments as done.
  • Addressed all the feedbacks
  • Fixed minor issues that would retrieve the wrong TargetCodeGenInfo instance
clang/lib/Basic/Targets.cpp
314

currently we don't have any plan for that

clang/lib/Basic/Targets/M68k.cpp
123

stack pointer is aliased to a7, but I'll add pc into the list.

clang/lib/CodeGen/TargetInfo.cpp
8083

Seems to work after I fixed a minor issue in getting the correct TargetCodeGenInfo. Will remove this comment.

Thanks for the changes. This looks good to me but I'll let others check again and approve.

myhsu updated this revision to Diff 309461.Dec 3 2020, 11:02 PM
  • Add support for M68060 sub-target
bruno accepted this revision.Dec 4 2020, 2:24 PM
bruno added a reviewer: bruno.

LGTM

This revision is now accepted and ready to land.Dec 4 2020, 2:24 PM
RKSimon added inline comments.Dec 9 2020, 2:30 AM
clang/lib/Basic/Targets/M68k.h
15

@myhsu This should probably be LLVM_CLANG_LIB_BASIC_TARGETS_M68K_H

jrtc27 added a subscriber: jrtc27.Dec 20 2020, 10:56 AM
jrtc27 added inline comments.
clang/lib/Basic/Targets/M68k.cpp
39–51

If we're in SysV psABI land, then the stack is 32-bit aligned.

If we're in actual ABI used by everyone out there, i.e. GCC's default, then it's only 16-bit aligned, and your integer types also have the wrong alignment, so you will get ABI incompatibility with GCC-built binaries as provided by distributions like Debian.

64

GCC's -mcpu excludes any M prefix and is just the number.

78–80

Where are these coming from? GCC only defines __m68k__.