User Details
- User Since
- Oct 28 2016, 2:19 AM (296 w, 8 h)
Thu, Jun 23
Remove conflict marker
Draft patch as some tests still fail.
Remove extraneous whitespace.
Make equal CHECK lines use WEBASSEMBLY:
Tue, Jun 21
rebase on top of D128282
Incorporate @asb semantic restrictions patch.
Generate test CHECK using update_cc_* script.
Wed, Jun 15
Update revision to be specific to externref only.
A different revision will be opened for further work on externref builtins and funcref.
May 30 2022
Still need to implement funcrefs support as function arguments.
Rebased the work on D126535. Added support for funcrefs as return values.
Still having problems with funcrefs as function arguments though. So, further work is required.
Not ready for review.
Thanks for the change. LGTM.
I will rebase my further changes on top of this.
Apr 11 2022
This still requires some changes, as there are a few unhandled cases where funcref attribute is not handled properly at the edge.
Moved the attribute handling to the border between clang and llvm.
In the frontend, there's no special handling of address spaces for funcref, but
this is added when converting the values to LLVM types.
A couple of weeks ago @asb pointed out we were missing an intrinsic for ref.is_null, so I got the work started. There's an issue with the polymorphic type for ref types, but I have seen this before and hopefully will get it fixed properly this time.
Mar 22 2022
Test wasm-funcref.c still fails as type attribute funcref semantics are not yet implemented. Not ready to land yet.
lgtm, thanks.
Mar 10 2022
Mar 5 2022
Thanks - all tests pass, landing again.
Mar 4 2022
@nokotan The test line should be something like: llvm-mc -mattr=+reference-types -triple=wasm32-unknown-unknown -filetype=obj -o - < %s | obj2yaml | FileCheck %s
However, I notice that MC/WebAssembly/tables.s is failing as well. Can you pls take a look?
Mar 3 2022
Thanks for your time working on the patch. It would be great to address @aardappel 's comments. The idea is to merge the function getGlobal with getTable, since they are so similar. I suppose into something like getGlobalOrTable. Add another switch case for wasm::WASM_SYMBOL_TYPE_TABLE to return the appropriate type.
Feb 25 2022
lgtm
Feb 22 2022
Thanks for the patch. In addition to what @sbc100 wrote, some further comments inline.
Feb 4 2022
@sbc100 might have an incorrect version of node as I keep getting /usr/bin/node: invalid value for --unhandled-rejections. Other than that there are no failures.
Maybe you can give it a go on your side just to be certain before landing?
Address last set of comments.
@sbc100 addresses your concerns.
Attempting to reland this with D118995 after fixing issues found.
Feb 3 2022
I confirmed with Anton K. in discord that indeed it's not possible to obtain a TLI outside a function, because a function can change the subtarget. Apparently ARM allows this with ARM vs Thumb functions being defined in the same file.
Jan 31 2022
Jan 29 2022
Move the removal of some lines setting table global type from D118121 to this revision.
Jan 28 2022
Remove non-NFC change.
Added [NFC] to the title, rebased on main and rechecked that tests are fine.
Jan 25 2022
@sbc100 I have now addressed all your comments, I feel like it is ready for review.
@sbc100 I addressed your questions regarding this part of the patch in D115749. I think it's ready for your review.
@sbc100 Thanks for the comments, I have split this into D118121 and D118122. I will now apply the comments from your reviews to the new revisions.
Jan 24 2022
Update the bug summary, no code changes.
Refactored the setting of the Wasm Symbol type into WasmSymbolSetType in WebAssemblyAsmPrinter.cpp and WebAssemblyMCInstLower.cpp. However, this forced us to move the function to WebAssemblyTypeUtilities.h and refactor some other type-related functions from WebAssemblyUtilities to WebAssemblyTypeUtilities.
Full patch for required refactoring and fix.
Jan 22 2022
Jan 17 2022
@sbc100 I think I understand where this is going wrong. As far as I understand emitExternalDecls should only be emitting .functype, etc directives for external symbols and yet it's doing so for all symbols in OutContext, even those which are not external. I will have to revisit my assumptions and take a look at the code again, since the current solution of running emitExternalDecls in onFinalization means all directives come at the end and that doesn't work because it complains if, for example a reference to bar comes before bars type declaration.
Refactored places where type related utility functions are and refactored some of the symbol type creation.
@sbc100 Unfortunately while adding some tests to this including testing private IR globals, which don't emit a .globl for the symbol I found a crash, which I now fixed and an issue with multiple globals in the same compilation unit. For example, two tables don't work because .tabletype is only emitted for the first. emitLinkage is called on each symbol in turn, and emitExternalDecls emits the tabletype for the symbol, however it only runs for the first symbol because emitExternalDecls only actually runs the first time it's called. This is very unfortunately and will need a bit of refactoring. I am working on it.
Jan 13 2022
Jan 12 2022
Ensure that we only call transform on Unevaluated Contexts, avoids the failure of a couple of vla tests.
Jan 11 2022
@efriedma I know it has been a long time, but are you still able to review this?
After a long hiatus on this bug, this is still failing on HEAD so lets get it fixed.
Rebased on top of main. All seems good.
Simplified code to emit labels for tables and tabletype directive.
Dec 21 2021
Remove isUndef check from code. I cannot find a codepath that generates an undef Idx at this point. All undef values end up being propogated down to the respective GEP. So simplifying and removing check.