[PATCH] D88466: [CSKY 1/n] Add basic stub or infra of csky backend

[David Chisnall via Phabricator via llvm-commits]

theraven added a comment.

Mostly boilerplate that looks fine, a few minor nits.



================
Comment at: llvm/lib/Target/CSKY/CSKYTargetMachine.cpp:31
+
+  // Specifies target lays out data in Big or Little endian form.
+  Ret += "e";
----------------
Minor nit, but hopefully anyone reading this knows what e means from reading the LangRef.  It would be more useful for this comment to give a summary of what is happening here specifically, for example 'CSky is always little endian with a 4-byte aligned stack'


================
Comment at: llvm/lib/Target/CSKY/CSKYTargetMachine.cpp:35
+  // S<size> Specifies the natural alignment of the stack in bits.
+  Ret += "-S32";
+
----------------
If the endian and stack alignment are the same for every instance of this back end, you don't need to add them separately.  The same is true for all of the things after the mangling flag.


================
Comment at: llvm/lib/Target/CSKY/CSKYTargetMachine.cpp:37
+
+  Ret += DataLayout::getManglingComponent(TT);
+
----------------
Does CSky really support all of the manglings that this can return?  Including Windows x86 mangling?  If not, please add some asserts to validate that it's a sane value.


================
Comment at: llvm/lib/Target/CSKY/CSKYTargetMachine.cpp:70
+
+static Reloc::Model getEffectiveRelocModel(const Triple &TT,
+                                           Optional<Reloc::Model> RM) {
----------------
Why make this a separate function instead of just writing this on line 84:
```
(RM.hasValue() ? *RM : Reloc::Static),
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88466/new/

https://reviews.llvm.org/D88466