[PATCH] [WebAssembly] Skeleton WebAssembly target

[JF Bastien]

================
Comment at: CODE_OWNERS.TXT:70
@@ +69,3 @@
+E: sunfish at mozilla.com
+D: WebAssembly Backend (lib/Target/WebAssembly/*)
+
----------------
Could you add me as well for WebAssembly? I could do a different change too...

================
Comment at: include/llvm/ADT/Triple.h:89
@@ -88,1 +88,3 @@
+    wasm32,     // WebAssembly with 32-bit pointers
+    wasm64,     // WebAssembly with 64-bit pointers
     LastArchType = shave
----------------
Add a mention of little-endian in both comment.

================
Comment at: include/llvm/ADT/Triple.h:156
@@ -153,2 +155,3 @@
     PS4,
-    LastOSType = PS4
+    WebAssembly,
+    LastOSType = WebAssembly
----------------
I'd rather spell the OS enum the same as its textual representation in IR (uppercase enum, lowercase string seems to be the trend). That's not a problem anymore since arch are `wasm32`/`wasm64`, so I'd rather have `WASM` as the OS.

================
Comment at: lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp:35
@@ +34,3 @@
+                                          unsigned RegNo) const {
+  llvm_unreachable("TODO: implement printRegName");
+}
----------------
`report_fatal_error` instead, here and below?

================
Comment at: lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCAsmInfo.cpp:24
@@ +23,3 @@
+
+WebAssemblyMCAsmInfo::WebAssemblyMCAsmInfo() {
+  AlignmentIsInBytes = false;
----------------
`PointerSize` is 4 by default, we need to set it for `wasm64`.
`CalleeSaveStackSlotSize` doesn't seem relevant, so the 4 default is fine?

(a bunch of others have OK defaults)

MaxInstLength doesn't seem relevant? Default is 4, but we don't have inline asm that matters because we're doing an AST. Maybe comment?
Same for MinInstAlignment.

(a bunch of others have OK defaults)

Should we do anything about `Code16Directive` / `Code32Directive` / `Code64Directive`? They don't make sense for us, so default is OK.

(a bunch of others have OK defaults)

What about `UseIntegratedAssembler`?

================
Comment at: lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCAsmInfo.cpp:25
@@ +24,3 @@
+WebAssemblyMCAsmInfo::WebAssemblyMCAsmInfo() {
+  AlignmentIsInBytes = false;
+  COMMDirectiveAlignmentIsInBytes = false;
----------------
It seems better to have `.align` be bytes? The historical confusion is annoying, so I'd just force people to use `.balign` and `.p2align` anyways, but I think in general folks expect `.align` to be bytes?

================
Comment at: lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCAsmInfo.cpp:29
@@ +28,3 @@
+  SupportsDebugInformation = true;
+  UseDataRegionDirectives = true;
+
----------------
Move this up, so it's in the same order as declared in the struct.

================
Comment at: lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCAsmInfo.cpp:35
@@ +34,3 @@
+  PrivateGlobalPrefix = ".L";
+  PrivateLabelPrefix = ".L";
+  HasDotTypeDotSizeDirective = false;
----------------
The defaults are `"L"`, which is the dot needed?

================
Comment at: lib/Target/WebAssembly/TargetInfo/WebAssemblyTargetInfo.cpp:28
@@ +27,3 @@
+  RegisterTarget<Triple::wasm64> Y(TheWebAssemblyTarget, "wasm64",
+                                   "WebAssembly 64-bit");
+}
----------------
add "(little endian)" to the description.

================
Comment at: lib/Target/WebAssembly/WebAssemblyFrameLowering.cpp:42
@@ +41,3 @@
+bool WebAssemblyFrameLowering::hasFP(const MachineFunction &MF) const {
+  llvm_unreachable("TODO: implement hasFP");
+}
----------------
`return_fatal_error` in this file too.

================
Comment at: lib/Target/WebAssembly/WebAssemblyFrameLowering.h:10
@@ +9,3 @@
+///
+/// \file
+/// \brief This class implements WebAssembly-specific bits of
----------------
CodingStandards.rst does say that `\file` and `\brief` should be in every file header. If we're trying to be on top of the standard then other files should also have this (it looks like you followed the local convention per-file, instead of blindly doing all-doxygen).

================
Comment at: lib/Target/WebAssembly/WebAssemblyFrameLowering.h:30
@@ +29,3 @@
+                            /*StackRealignable=*/true) {}
+
+  void
----------------
Could you add a FIXME to implement `assignCalleeSavedSpillSlots` and `getCalleeSavedSpillSlots`? It looks like it's something we'll eventually want.

================
Comment at: lib/Target/WebAssembly/WebAssemblyISelDAGToDAG.cpp:37
@@ +36,3 @@
+
+  bool ForCodeSize;
+
----------------
`OptimizeForSize` instead?

================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrAtomics.td:40
@@ +39,3 @@
+
+// Load-exclusives.
+
----------------
Redundant.

================
Comment at: lib/Target/WebAssembly/WebAssemblyRegisterInfo.h:27
@@ +26,3 @@
+class WebAssemblyRegisterInfo final {
+private:
+  const Triple &TT;
----------------
No need for `private:` here.

================
Comment at: lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp:48
@@ +47,3 @@
+                               ? "e-p:64:64-i64:64-v128:8:128-n32:64-S128"
+                               : "e-p:32:32-i64:64-v128:8:128-n32:64-S128",
+                        TT, CPU, FS, Options, RM, CM, OL),
----------------
There doesn't seem to be a default mangling for datalayout, ELF seems like what we want: `m:e`.
These are the defaults, but would be nice to specify: `f32:32-f64:64` ?

http://reviews.llvm.org/D10569

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/