[PATCH] D52748: [WebAssembly] LSDA info generation

[Derek Schuff via Phabricator via llvm-commits]

dschuff added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/EHStreamer.cpp:610
+  if (IsWasm)
+    return GCCETSym;
+
----------------
aheejin wrote:
> sbc100 wrote:
> > This seems a little off,  could remove that requirement?
> > 
> > If we can't then perhaps invert this check we we only have a single return statement?
> > 
> > ```
> > if (!IsWasm)
> >   Asm->EmitAlignment(2);
> > return GCCETSym;
> > ```
> Yes it seemed a little off to me :( We can remove it, but it requires more aggressive refactoring. So basically the problem is, while it is better to move wasm-specific functionalities to `WasmException` class like as I did for `WasmException::computeCallSiteTable`, wasm can completely reuse the existing `emitExceptionTable`, except it requires to emit something before the final alignment directive. To refactor this, we should probably split this `emitExceptionTable` into 2-3 different functions, and make `WasmException` call only relevant functions from them. What do you think? Do you think it's worth it?
If it causes a lot of ugliness, there's actually no reason we need to emit the size before the alignment; it could go after. The only difference is whether the calculated size of the object includes or doesn't include the padding that the align directive might emit. (This would mean that when emit the table in the object file, the resulting segment might contain up to 3 bytes of padding on the end instead of having a gap between the segments, but that's not the end of the world).


Repository:
  rL LLVM

https://reviews.llvm.org/D52748