[PATCH] [WebAssembly] Skeleton WebAssembly target

JF Bastien jfb at chromium.org
Mon Jun 29 16:16:50 PDT 2015


No strong preference on the last comments, would be nice to fix, but lgtm either way.


================
Comment at: lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp:35
@@ +34,3 @@
+                                          unsigned RegNo) const {
+  llvm_unreachable("TODO: implement printRegName");
+}
----------------
sunfish wrote:
> jfb wrote:
> > `report_fatal_error` instead, here and below?
> This code is totally unusable without this function, so it doesn't really matter. Crashing is better because we get a stack trace to where we need to write code.
`llvm_unreachable` will only crash in debug, though?

================
Comment at: lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCAsmInfo.cpp:25
@@ +24,3 @@
+WebAssemblyMCAsmInfo::WebAssemblyMCAsmInfo() {
+  AlignmentIsInBytes = false;
+  COMMDirectiveAlignmentIsInBytes = false;
----------------
sunfish wrote:
> jfb wrote:
> > 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?
> I'm all for forcing people to use .p2align/.balign; is that an option? Otherwise, looking at LLVM's other targets, making .align be .p2align seems fairly popular.
I don't think it's an option... Asking folks around me they expect `.balign` behavior.

http://reviews.llvm.org/D10569

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






More information about the llvm-commits mailing list