[PATCH] D83729: [WebAssembly] 64-bit (function) pointer fixes.

Mon Jul 13 17:41:09 PDT 2020

sbc100 added inline comments.

================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:689
+  // diagnose() does not call exit() if there's a handler.
+  exit(1);
 }
----------------
aardappel wrote:
> sbc100 wrote:
> > This seems unrelated..  and untested?
> This is something I ran into while testing this code. It not exiting caused problems further down the line in the wasm backend which it shouldn't.. this is strictly a bug in non-wasm code, but didn't want to touch that. Not sure how you'd even test this. Do you want this in a review by itself?
Ok,  I guess I don't really understand the comment.    Is diagnose supposed to call exit?   Should we have a TODO here if so?   I'm not sure what you mean by handler either.

================
Comment at: llvm/test/CodeGen/WebAssembly/pointer64.ll:1
+; RUN: llc < %s -asm-verbose=false -O2 | FileCheck %s
+; RUN: llc < %s -asm-verbose=false -O2 --filetype=obj | obj2yaml | FileCheck --check-prefix=YAML %s
----------------
aardappel wrote:
> sbc100 wrote:
> > Don't we already have a function pointer test that we can update to test both arches?  
> This tests things very specific to wasm64 (e.g. the truncation), so I prefer a test that focuses just on what's new.
Ok, you about we give it a more specific name then.  `test_call_indirect_wasm64.ll`?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83729