M680x0 / M680x0-mono-repo

Mono-Repo LLVM Backend for Motorola M68000 (Work in Progress)
42 stars 5 forks source link

Warnings while compiling llvm/lib/Support/Triple.cpp #1

Closed p2k closed 4 years ago

p2k commented 4 years ago
[10/2962] Building CXX object lib\Support\CMakeFiles\LLVMSupport.dir\Triple.cpp.obj
D:\workspace\LLVM-M680x0\llvm\lib\Support\Triple.cpp(22,11): warning: enumeration value 'm68k' not handled in switch [-Wswitch]
  switch (Kind) {
          ^
D:\workspace\LLVM-M680x0\llvm\lib\Support\Triple.cpp(662,11): warning: enumeration value 'm68k' not handled in switch [-Wswitch]
  switch (T.getArch()) {
          ^
D:\workspace\LLVM-M680x0\llvm\lib\Support\Triple.cpp(1241,11): warning: enumeration value 'm68k' not handled in switch [-Wswitch]
  switch (Arch) {
          ^
D:\workspace\LLVM-M680x0\llvm\lib\Support\Triple.cpp(1320,11): warning: enumeration value 'm68k' not handled in switch [-Wswitch]
  switch (getArch()) {
          ^
D:\workspace\LLVM-M680x0\llvm\lib\Support\Triple.cpp(1386,11): warning: enumeration value 'm68k' not handled in switch [-Wswitch]
  switch (getArch()) {
          ^
5 warnings generated.

Compiling on Windows via clang-cl, but this should not matter. Can't say if this warning is critical, but probably easy to fix. I can't submit a patch, because I don't know the exact intention of this.

mshockwave commented 4 years ago

Hi @p2k thanks for the feedback. As you probably have noticed, the migration process is still under heavily development (we haven't even fixed all of the building errors). So any patch is absolutely welcome!

p2k commented 4 years ago

(A bit off topic, but please allow me) I have begun working on the compile errors by tracing down the changes between the state of the project before the merge and after - which is sort of a nice puzzle also because I haven't worked on LLVM before. There is a chance I fix things differently than you and also that I might make mistakes; I am using my best educated guesses, though.

Could you do me a favor and point me to the right guide so I can make proper pull requests to the project and get my commit messages formatted right etc.?

p2k commented 4 years ago

(Back on topic, sorry again) Is the target called "m680x0" or "m68k" in regards to the triple? You kind of have to make up your mind, because there should only be one canonical name for the triple; all others will become aliases. I would suggest "m68k" since it is shorter and more known and likely used as the operating system's identifier - after all you have several lines in llvm/cmake/config.guess that point to m68k and none that point to m680x0.

mshockwave commented 4 years ago

(A bit off topic, but please allow me) I have begun working on the compile errors by tracing down the changes between the state of the project before the merge and after - which is sort of a nice puzzle also because I haven't worked on LLVM before. There is a chance I fix things differently than you and also that I might make mistakes; I am using my best educated guesses, though.

That would be awesome.

Could you do me a favor and point me to the right guide so I can make proper pull requests to the project and get my commit messages formatted right etc.?

We don't have for this project. But just follow LLVM's guidelines.

p2k commented 4 years ago

Okay, I have just seen you pushed your newest fixes. Turns out you solved the problems nearly the same way as me, which is pretty good as it reassures me to be on the right track.

Can you answer my second question about the target triple?

mshockwave commented 4 years ago

Thanks for your additional improvement on top of my fixes. Regarding the target triple, I don't really have any strong opinion, what do you think about that @m4yers @glaubitz ?

glaubitz commented 4 years ago

m68k-linux-gnu is fine and is what most users would expect, it's also commonly used on distributions like Debian.

As for the backend name, I think M680x0 should stay as it fits the naming scheme of LLVM to use longer names for the backends such as SystemZ.

@p2k And thanks for your help!

m4yers commented 4 years ago

Yes, m68k is a standard target name so this backend should understand it.