[PATCH] D65463: [WebAssembly] Fix conflict between ret legalization and sjlj

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added a subscriber: tlively.
aheejin added a comment.

Thank you for fixing this! This is a long-known bug <https://bugs.llvm.org/show_bug.cgi?id=33824> we haven't fixed so far, because wasm C++ frontend lowers struct returns into sret arguments in the frontend, so we don't usually encounter this bug. Just out of curiosity, how did you discover this bug? (The reporter of this bug <https://bugs.llvm.org/show_bug.cgi?id=33824> found it while using the Rust frontend, I heard)

I like this approach. I prefer the option 2 among the three options you described: 1 is redundantly doing what ISel is already doing, and 3 makes LLVM-backend generated code incorrect so we have to rely on the other tool to fix it, which seems not very desirable. As you said, having to add an extra argument to the interface of `CanLowerReturn` is a bit sad but I don't really mind that if others are OK with it.

@dschuff @tlively Any more opinions?



================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:818
+    return true;
+  }
+
----------------
Real nit: In this file it seems we mostly don't add braces to one-line ifs. The same for the if clause below.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65463