[PATCH] D139010: [clang][WebAssembly] Implement support for table types and builtins

[Aaron Ballman via Phabricator via llvm-commits]

aaron.ballman added inline comments.


================
Comment at: clang/docs/LanguageExtensions.rst:2288
+argument is the index to which to store the value into, and the
+third argument is a value of reference type to store in the table.
+It returns nothing.
----------------
This sounds like any reference type will work, e.g.,
```
static __externref_t table[0];
void func(int i) {
  int &ref = i;
  __builtin_wasm_table_set(table, i, ref);
}
```
so might want to say "value of ``_externref_t`` type" instead?


================
Comment at: clang/docs/LanguageExtensions.rst:2291-2292
+
+.. code-block:: c++
+  static __externref_t table[0];
+  extern __externref_t JSObj;
----------------
Sphinx will bark otherwise.


================
Comment at: clang/docs/LanguageExtensions.rst:2303
+This builtin function is the counterpart to ``__builtin_wasm_table_set``
+and load a value from a WebAssembly table of reference typed values.
+It takes 2 arguments.
----------------



================
Comment at: clang/docs/LanguageExtensions.rst:2309-2310
+
+.. code-block:: c++
+  static __externref_t table[0];
+  
----------------
Sphinx will bark otherwise.


================
Comment at: clang/docs/LanguageExtensions.rst:2321
+This builtin function returns the size of the WebAssembly table.
+Takes as argument the table and returns an integer with the current
+table size.
----------------
Returns an `int`? `size_t`? Something else?


================
Comment at: clang/docs/LanguageExtensions.rst:2324-2325
+
+.. code-block:: c++
+  typedef void (*__funcref funcref_t)();
+  static __funcref table[0];
----------------



================
Comment at: clang/docs/LanguageExtensions.rst:2340
+It takes three arguments. The first argument is the WebAssembly table 
+to grow. The second argument is the reference typed value to store in 
+the new table entries (the initialization value), and the third argument
----------------
Same suggestion here as above regarding arbitrary reference types.


================
Comment at: clang/docs/LanguageExtensions.rst:2345-2346
+
+.. code-block:: c++
+  typedef void (*__funcref funcref_t)();
+  static __funcref table[0];
----------------



================
Comment at: clang/docs/LanguageExtensions.rst:2360-2361
+
+This builtin function sets all the entries of a WebAssembly table to a given 
+reference typed value. It takes four arguments. The first argument is 
+the WebAssembly table, the second argument is the index that starts the 
----------------
Same suggestion here as above regarding arbitrary reference types.


================
Comment at: clang/docs/LanguageExtensions.rst:2364-2365
+range, the third argument is the value to set in the new entries, and 
+the fourth and the last argument is the size of the range. It returns 
+nothing.
+
----------------
What happens if the range is invalid for the table size? e.g., the user never called `__builtin_wasm_table_grow` before calling `__builtin_wasm_table_fill`?


================
Comment at: clang/docs/LanguageExtensions.rst:2367-2368
+
+.. code-block:: c++
+  static __externref_t table[0];
+
----------------



================
Comment at: clang/docs/LanguageExtensions.rst:2385
+source index from there the copy starts, and the fifth and last argument
+is the number of elements to copy. It returns nothing.
+
----------------
Similar question here as above -- what happens when the range given is invalid for either one or both of the table arguments?


================
Comment at: clang/test/SemaCXX/wasm-refs-and-tables.cpp:35
+task<__externref_t[]> g() {
+  co_return table;
+}
----------------
pmatos wrote:
> @aaron.ballman I tried and failed to create a good testcase for co_return. However creating coroutines seems to be an stdlib thing which I am not sure how to test here. Do you have any suggestions here?
```
#include "Inputs/std-coroutine.h"

using std::suspend_always;

struct promise_table {
  __externref_t[] get_return_object();
  suspend_always initial_suspend();
  suspend_always final_suspend() noexcept;
  void return_value(__externref_t[]);
  void unhandled_exception();
};

template <typename... T>
struct std::coroutine_traits<__externref_t[], T...> { using promise_type = promise_table; };

static __externref_t table[0];
__externref_t[] func() {
  co_return table; // Cannot return a WebAssembly table?
}
```
Perhaps something along these lines?


================
Comment at: llvm/include/llvm/CodeGen/WasmAddressSpaces.h:44
+} // namespace llvm
\ No newline at end of file

----------------
aaron.ballman wrote:
> Add newline
Still need to add this newline back, btw.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139010