[PATCH] D88389: [M68K] (Patch 3/8) Basic infrastructures and target description files
Craig Topper via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 6 20:51:02 PDT 2020
craig.topper added inline comments.
================
Comment at: llvm/include/llvm/ADT/Triple.h:50
- arm, // ARM (little endian): arm, armv.*, xscale
- armeb, // ARM (big endian): armeb
- aarch64, // AArch64 (little endian): aarch64
- aarch64_be, // AArch64 (big endian): aarch64_be
- aarch64_32, // AArch64 (little endian) ILP32: aarch64_32
- arc, // ARC: Synopsys ARC
- avr, // AVR: Atmel AVR microcontroller
- bpfel, // eBPF or extended BPF or 64-bit BPF (little endian)
- bpfeb, // eBPF or extended BPF or 64-bit BPF (big endian)
- csky, // CSKY: csky
- hexagon, // Hexagon: hexagon
- mips, // MIPS: mips, mipsallegrex, mipsr6
- mipsel, // MIPSEL: mipsel, mipsallegrexe, mipsr6el
- mips64, // MIPS64: mips64, mips64r6, mipsn32, mipsn32r6
- mips64el, // MIPS64EL: mips64el, mips64r6el, mipsn32el, mipsn32r6el
- msp430, // MSP430: msp430
- ppc, // PPC: powerpc
- ppc64, // PPC64: powerpc64, ppu
- ppc64le, // PPC64LE: powerpc64le
- r600, // R600: AMD GPUs HD2XXX - HD6XXX
- amdgcn, // AMDGCN: AMD GCN GPUs
- riscv32, // RISC-V (32-bit): riscv32
- riscv64, // RISC-V (64-bit): riscv64
- sparc, // Sparc: sparc
- sparcv9, // Sparcv9: Sparcv9
- sparcel, // Sparc: (endianness = little). NB: 'Sparcle' is a CPU variant
- systemz, // SystemZ: s390x
- tce, // TCE (http://tce.cs.tut.fi/): tce
- tcele, // TCE little endian (http://tce.cs.tut.fi/): tcele
- thumb, // Thumb (little endian): thumb, thumbv.*
- thumbeb, // Thumb (big endian): thumbeb
- x86, // X86: i[3-9]86
- x86_64, // X86-64: amd64, x86_64
- xcore, // XCore: xcore
- nvptx, // NVPTX: 32-bit
- nvptx64, // NVPTX: 64-bit
- le32, // le32: generic little-endian 32-bit CPU (PNaCl)
- le64, // le64: generic little-endian 64-bit CPU (PNaCl)
- amdil, // AMDIL
- amdil64, // AMDIL with 64-bit pointers
- hsail, // AMD HSAIL
- hsail64, // AMD HSAIL with 64-bit pointers
- spir, // SPIR: standard portable IR for OpenCL 32-bit version
- spir64, // SPIR: standard portable IR for OpenCL 64-bit version
- kalimba, // Kalimba: generic kalimba
- shave, // SHAVE: Movidius vector VLIW processors
- lanai, // Lanai: Lanai 32-bit
- wasm32, // WebAssembly with 32-bit pointers
- wasm64, // WebAssembly with 64-bit pointers
+ arm, // ARM (little endian): arm, armv.*, xscale
+ armeb, // ARM (big endian): armeb
----------------
Don't reformat the other lines
================
Comment at: llvm/lib/Support/Triple.cpp:76
case wasm64: return "wasm64";
+ case m680x0:
+ return "m680x0";
----------------
I think this switch is in alphabetical order.
Put the return on the previous line for consistency
================
Comment at: llvm/lib/Support/Triple.cpp:158
case csky: return "csky";
+ case m680x0:
+ return "m68k";
----------------
Put return on previous line for consistency
================
Comment at: llvm/lib/Support/Triple.cpp:275
return StringSwitch<Triple::ArchType>(Name)
- .Case("aarch64", aarch64)
- .Case("aarch64_be", aarch64_be)
- .Case("aarch64_32", aarch64_32)
- .Case("arc", arc)
- .Case("arm64", aarch64) // "arm64" is an alias for "aarch64"
- .Case("arm64_32", aarch64_32)
- .Case("arm", arm)
- .Case("armeb", armeb)
- .Case("avr", avr)
- .StartsWith("bpf", BPFArch)
- .Case("mips", mips)
- .Case("mipsel", mipsel)
- .Case("mips64", mips64)
- .Case("mips64el", mips64el)
- .Case("msp430", msp430)
- .Case("ppc64", ppc64)
- .Case("ppc32", ppc)
- .Case("ppc", ppc)
- .Case("ppc64le", ppc64le)
- .Case("r600", r600)
- .Case("amdgcn", amdgcn)
- .Case("riscv32", riscv32)
- .Case("riscv64", riscv64)
- .Case("hexagon", hexagon)
- .Case("sparc", sparc)
- .Case("sparcel", sparcel)
- .Case("sparcv9", sparcv9)
- .Case("systemz", systemz)
- .Case("tce", tce)
- .Case("tcele", tcele)
- .Case("thumb", thumb)
- .Case("thumbeb", thumbeb)
- .Case("x86", x86)
- .Case("x86-64", x86_64)
- .Case("xcore", xcore)
- .Case("nvptx", nvptx)
- .Case("nvptx64", nvptx64)
- .Case("le32", le32)
- .Case("le64", le64)
- .Case("amdil", amdil)
- .Case("amdil64", amdil64)
- .Case("hsail", hsail)
- .Case("hsail64", hsail64)
- .Case("spir", spir)
- .Case("spir64", spir64)
- .Case("kalimba", kalimba)
- .Case("lanai", lanai)
- .Case("shave", shave)
- .Case("wasm32", wasm32)
- .Case("wasm64", wasm64)
- .Case("renderscript32", renderscript32)
- .Case("renderscript64", renderscript64)
- .Case("ve", ve)
- .Case("csky", csky)
- .Default(UnknownArch);
+ .Case("aarch64", aarch64)
+ .Case("aarch64_be", aarch64_be)
----------------
Are there tabs on these lines now?
================
Comment at: llvm/lib/Support/Triple.cpp:1274
case llvm::Triple::mipsel:
+ case llvm::Triple::m680x0:
case llvm::Triple::nvptx:
----------------
Should this be before mips alphabetically? That would be consistent with line 699.
================
Comment at: llvm/lib/Support/Triple.cpp:1359
case Triple::mipsel:
+ case Triple::m680x0:
case Triple::nvptx:
----------------
alphabetize
================
Comment at: llvm/lib/Support/Triple.cpp:1409
case Triple::msp430:
+ case Triple::m680x0:
case Triple::r600:
----------------
alphabetize
================
Comment at: llvm/lib/Target/LLVMBuild.txt:38
WebAssembly
+ M680x0
X86
----------------
Alphabetize
================
Comment at: llvm/lib/Target/M680x0/M680x0.h:26
+
+/// This pass converts a legalized DAG into a M680x0-specific DAG, ready for
+/// instruction scheduling.
----------------
These declarations should probably go with the patch that has their definitions.
================
Comment at: llvm/lib/Target/M680x0/M680x0InstrInfo.td:381
+def IsNotPIC : Predicate<"!TM.isPositionIndependent()">;
+def IsM68000 : Predicate<"!Subtarget.IsM68000()">;
+def IsM68010 : Predicate<"!Subtarget.IsM68010()">;
----------------
Why is there a ! in the string? Doesn't that mean "not"?
================
Comment at: llvm/lib/Target/M680x0/M680x0InstrInfo.td:416
+
+// NOTE Though this CP is not stricly necessarily it will simplify instruciton
+// definitions
----------------
stricly -> strictly
================
Comment at: llvm/lib/Target/M680x0/M680x0InstrInfo.td:459
+//
+// TODO #48 you need to evaluate whether splitting one big shift(or rotate)
+// into a few smaller is faster than doing a move, if so do custom lowering
----------------
This #48 a meaningful number in a list somewhere?
================
Comment at: llvm/lib/Target/M680x0/M680x0InstrInfo.td:474
+ if (ExtType == ISD::EXTLOAD)
+ return LD->getAlignment() >= 2 && !LD->isVolatile();
+ return false;
----------------
isVolatile here should be isSimple. This code looks to have originated in X86 and its been changed there
================
Comment at: llvm/lib/Target/M680x0/M680x0InstrInfo.td:484
+ if (ExtType == ISD::EXTLOAD)
+ return LD->getAlignment() >= 4 && !LD->isVolatile();
+ return false;
----------------
Same here
================
Comment at: llvm/lib/Target/M680x0/TargetInfo/M680x0DummyInfo.cpp:2
+// WARNING: This file is created only for patch review
+// In order to build this patch w/o errors, some libraris like
+// LLVM<Target>CodeGen need to be present and some APIs
----------------
libraris ->libraries?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88389/new/
https://reviews.llvm.org/D88389
More information about the llvm-commits
mailing list