[PATCH] D52748: [WebAssembly] LSDA info generation

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/EHStreamer.cpp:610
+  if (IsWasm)
+    return GCCETSym;
+
----------------
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?


================
Comment at: test/CodeGen/WebAssembly/eh-lsda.ll:44
+; CHECK-NEXT:   .int8  0                       # @TType Encoding = absptr
+; CHECK-NEXT:   .uleb128 .Lttbase0-.Lttbaseref0
+; CHECK-NEXT: .Lttbaseref0:
----------------
sbc100 wrote:
> Ha.. I didn't know that assembler could be used to encode LEBs like this.
> 
> I'm assuming this assembly value (i.e. doesn't generate a relocations?)
Yeah it doesn't generate a relocation.


Repository:
  rL LLVM

https://reviews.llvm.org/D52748