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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG55aeb23fe008: [clang][WebAssembly] Implement support for table types and builtins (authored by pmatos). Changed prior to commit:

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

pmatos added a comment. In D139010#4400303 , @aaron.ballman wrote: > Spotted a typo in the docs, but otherwise LGTM (thanks for your patience > while I was out last week). Thanks for your time one this. Repository: rG LLVM Github Monorepo CHANGES

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

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Spotted a typo in the docs, but otherwise LGTM (thanks for your patience while I was out last week). Comment at: clang/docs/LanguageExtensions.rst:2310

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

pmatos updated this revision to Diff 526578. pmatos added a comment. Apply clang-format. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files: clang/docs/LanguageExtensions.rst

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

pmatos updated this revision to Diff 526539. pmatos added a comment. Remove test portion attempting to test coroutines. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files:

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

pmatos updated this revision to Diff 526436. pmatos marked an inline comment as done. pmatos added a comment. Address the comments wrt the documentation. Added link to Wasm spec. @aaron.ballman What do you think? I added the reference to the top level of the WebAssembly section, rather than to

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

pmatos marked 3 inline comments as done. pmatos 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.

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

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. pmatos wrote: >

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

pmatos 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. aaron.ballman wrote: > This

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

pmatos updated this revision to Diff 524710. pmatos marked 8 inline comments as done. pmatos added a comment. Address remaining comments. Make table.size builtin return size_t. Still remaining to be address is co_return on tables. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

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

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

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

pmatos added a comment. In D139010#4350869 , @tlively wrote: > It looks like the LLVM-side changes are generally moving Wasm type > classification functions to a more global location. Since no other backend > should care about these things, it would be

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

pmatos updated this revision to Diff 523741. pmatos added a comment. Remove modifications from Type.h and move them to WebAssembly files. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files:

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

tlively added a comment. It looks like the LLVM-side changes are generally moving Wasm type classification functions to a more global location. Since no other backend should care about these things, it would be better if we could get away without these changes. Comment at:

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

tlively added a comment. Thanks for the ping, will take a look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 ___ cfe-commits mailing list

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

pmatos marked an inline comment as done. pmatos added a comment. @tlively @aaron.ballman pinging both to get final reviews on the LLVM and Clang parts respectively. It would be great to have this merged soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

pmatos marked an inline comment as done. pmatos added a comment. I think we are almost there. @aaron.ballman what do you think? Comment at: clang/test/Sema/wasm-refs-and-tables.c:81 + + funcref_t func = __builtin_wasm_ref_null_func(0); // expected-error {{too many arguments

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

pmatos updated this revision to Diff 517535. pmatos added a comment. Add documentation for table builtins. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files: clang/docs/LanguageExtensions.rst

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

pmatos added inline comments. Comment at: clang/test/SemaCXX/wasm-refs-and-tables.cpp:35 +task<__externref_t[]> g() { + co_return table; +} @aaron.ballman I tried and failed to create a good testcase for co_return. However creating coroutines seems to be an

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

pmatos updated this revision to Diff 517525. pmatos marked an inline comment as done. pmatos added a comment. Quick fixup of test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files:

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

pmatos marked an inline comment as done. pmatos added inline comments. Comment at: clang/test/Sema/wasm-refs-and-tables.c:17 +static __externref_t t6[] = {0}; // expected-error {{only zero-length WebAssembly tables are currently supported}} +__externref_t t7[0]; //

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

pmatos updated this revision to Diff 517521. pmatos marked 2 inline comments as done. pmatos added a comment. Update a few more tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files:

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

pmatos marked 2 inline comments as done. pmatos added inline comments. Comment at: clang/include/clang/Basic/BuiltinsWebAssembly.def:205-210 +TARGET_BUILTIN(__builtin_wasm_table_set, "viii", "t", "reference-types") +TARGET_BUILTIN(__builtin_wasm_table_get, "iii", "t",

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

aaron.ballman added inline comments. Comment at: clang/test/Sema/wasm-refs-and-tables.c:17 +static __externref_t t6[] = {0}; // expected-error {{only zero-length WebAssembly tables are currently supported}} +__externref_t t7[0]; // expected-error {{WebAssembly table

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

pmatos updated this revision to Diff 517478. pmatos marked 6 inline comments as done. pmatos added a comment. Complete fixing tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files:

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

pmatos added a comment. Thanks for the comments - I am working on addressing these at the moment. The LLVM part of the patch is just some refactoring and therefore should be pretty trivial, pinging @tlively in case he has some time. Comment at:

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

pmatos updated this revision to Diff 517166. pmatos marked 3 inline comments as done. pmatos added a comment. Fix Wasm table tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files:

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

pmatos marked an inline comment as done. pmatos added inline comments. Comment at: clang/test/Sema/builtins-wasm.c:12-13 + __builtin_wasm_table_size(table, table);// expected-error {{too many arguments to function call, expected 1, have 2}} + void *a =

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

aaron.ballman added a comment. This will also need someone from the LLVM side to look at the LLVM-specific changes. Most of my comments were focused on the behavior of test cases, but there may be more comments coming for the code changes once I've got a better handle on the test behavior.

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

pmatos added a comment. @aaron.ballman Any further comments on this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 ___ cfe-commits mailing list

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

pmatos added inline comments. Comment at: clang/test/SemaCXX/wasm-refs-and-tables.cpp:16-17 +__externref_t **t2; // expected-error {{pointer to WebAssembly reference type is not allowed}} +__externref_t **t3; // expected-error {{pointer to WebAssembly

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

pmatos updated this revision to Diff 506552. pmatos marked 4 inline comments as done. pmatos added a comment. Address concerns about table subscripting. Add warnings for that. Address a few other smaller comments like testing for table comparisons. Repository: rG LLVM Github Monorepo

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

pmatos updated this revision to Diff 506521. pmatos added a comment. Remove references to table subscript. Simplify patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files:

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

pmatos updated this revision to Diff 506499. pmatos added a comment. Rebase on main. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files: clang/include/clang/AST/Expr.h

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

aaron.ballman added a comment. In D139010#4159307 , @tlively wrote: > In D139010#4158540 , @aaron.ballman > wrote: > >> Roping in @jfb because I know he's been heavily involve in WebAssembly in >> the past and

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

tlively added a comment. In D139010#4158540 , @aaron.ballman wrote: > Roping in @jfb because I know he's been heavily involve in WebAssembly in the > past and he may have ideas/opinions. > > High-level question: are externref, funcref, and table types

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

pmatos updated this revision to Diff 501143. pmatos marked an inline comment as done. pmatos added a comment. Address comments regarding tables and remove some unused code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/

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

aaron.ballman added a reviewer: jfb. aaron.ballman added a subscriber: jfb. aaron.ballman added a comment. Roping in @jfb because I know he's been heavily involve in WebAssembly in the past and he may have ideas/opinions. High-level question: are externref, funcref, and table types all part of

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

pmatos marked 3 inline comments as done. pmatos added inline comments. Comment at: clang/include/clang/AST/Stmt.h:456 - class ArrayOrMatrixSubscriptExprBitfields { + class AMTSubscriptExprBitfields { friend class ArraySubscriptExpr; aaron.ballman wrote:

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

pmatos updated this revision to Diff 501110. pmatos marked an inline comment as done. pmatos added a comment. Address the comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files:

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

aaron.ballman added a reviewer: erichkeane. aaron.ballman added inline comments. Comment at: clang/include/clang/AST/Stmt.h:456 - class ArrayOrMatrixSubscriptExprBitfields { + class AMTSubscriptExprBitfields { friend class ArraySubscriptExpr; I am not

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

pmatos updated this revision to Diff 497633. pmatos added a comment. Rebased on current HEAD. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files: clang/include/clang/AST/Expr.h

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

pmatos updated this revision to Diff 493264. pmatos added a comment. Rebase on main and fix a test diagnostic expectation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files:

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

pmatos added a comment. @aaron.ballman Thanks for all the comments, I have now finished addressing those. What do you think of the current patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010

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

pmatos updated this revision to Diff 491790. pmatos added a comment. Add missing newline at end of file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files: clang/include/clang/AST/Expr.h

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

pmatos marked an inline comment as done. pmatos added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11827 +def err_wasm_builtin_arg_must_be_integer_type : Error < + "%ordinal0 argument must be an integer">; } // end of sema component.

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

pmatos updated this revision to Diff 491784. pmatos added a comment. Simplify and add tests for remaining diagnostics Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files:

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

pmatos updated this revision to Diff 491707. pmatos added a comment. Further tests for use of tables in conditional branches. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files:

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

pmatos updated this revision to Diff 491608. pmatos added a comment. Finish fixing tests after dianostic message changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files:

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

pmatos updated this revision to Diff 491468. pmatos added a comment. Fix some tests whose diagnostics changed Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files: clang/include/clang/AST/Expr.h

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

pmatos updated this revision to Diff 491353. pmatos marked 7 inline comments as done. pmatos added a comment. Address more comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files:

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

pmatos marked 2 inline comments as done. pmatos added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11797 +def err_typecheck_wasm_table_must_have_zero_length : Error< + "only zero-length WebAssembly tables are currently supported">; +def

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

asb added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11827 +def err_wasm_builtin_arg_must_be_integer_type : Error < + "%ordinal0 argument must be an integer">; } // end of sema component. aaron.ballman wrote: > I'm a bit

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

pmatos updated this revision to Diff 490208. pmatos added a comment. Remove some unnecessary diagnostics. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files: clang/include/clang/AST/Expr.h

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

pmatos marked 5 inline comments as done. pmatos added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2171-2176 def err_reference_bind_to_vector_element : Error< "%select{non-const|volatile}0 reference cannot bind to vector element">; def

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

pmatos updated this revision to Diff 489468. pmatos added a comment. Updating tests after some changes to previous diagnostics. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files:

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

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:1511 + /// element type + QualType getWasmTableType(QualType EltTy) const; + Ah, I suggested `WebAssembly` in another review, but `Wasm` is also fine by me -- just pick a

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

pmatos updated this revision to Diff 487747. pmatos added a comment. Fix test broken after recent change to opt call. Rebase again on main. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files:

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

pmatos updated this revision to Diff 487702. pmatos added a comment. Rebase on top of main. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files: clang/include/clang/AST/ASTContext.h

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

pmatos added a comment. This is ready for review. RFC has been published here: https://discourse.llvm.org/t/rfc-webassembly-reference-types-in-clang/66939 Thanks to @tlively and @asb for the reviews. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

pmatos updated this revision to Diff 479265. pmatos added a comment. Remove unnecessary attribute that lingered around after some refactoring. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010 Files:

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

pmatos added a comment. Subsumes D124162 and D123510 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010

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

pmatos planned changes to this revision. pmatos added a comment. Please wait to review until the RFC (coming soon) is published. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139010/new/ https://reviews.llvm.org/D139010

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

pmatos created this revision. Herald added subscribers: wingo, ecnelises, sunfish, hiraditya, jgravelle-google, sbc100, dschuff. Herald added a reviewer: aaron.ballman. Herald added a project: All. pmatos requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits,