[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG890146b19206: [WebAssembly] Initial support for reference type externref in clang (authored by pmatos, committed by vitalybuka). Repository: rG

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

vitalybuka updated this revision to Diff 498550. vitalybuka added a comment. clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/ https://reviews.llvm.org/D122215 Files: clang/include/clang/AST/ASTContext.h

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 497630. pmatos added a comment. Rebased against HEAD. Waiting for @vitalybuka go ahead to land. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/ https://reviews.llvm.org/D122215 Files:

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

asb added a comment. In D122215#4119038 , @vitalybuka wrote: > In D122215#4118516 , @asb wrote: > >> @pmatos and I have tried and failed to reproduce the assert in the stage3 >> msan build locally (both of us

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

vitalybuka added a comment. In D122215#4118516 , @asb wrote: > @pmatos and I have tried and failed to reproduce the assert in the stage3 > msan build locally (both of us on separate machines, different environments). > We've reached out to @vitalybuka

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

asb added a comment. @pmatos and I have tried and failed to reproduce the assert in the stage3 msan build locally (both of us on separate machines, different environments). We've reached out to @vitalybuka via email for any help in reproducing, because we're somewhat stumped at the moment.

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

vitalybuka updated this revision to Diff 494999. vitalybuka added a comment. as reverted Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/ https://reviews.llvm.org/D122215 Files: clang/include/clang/AST/ASTContext.h

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

jgorbe added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:4012-4018 + if (Target->getTriple().isWasm() && Target->hasFeature("reference-types")) { +#define WASM_REF_TYPE(Name, MangledName, Id, SingletonId, AS) \ + if (BuiltinType::Id ==

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGeb66833d1957: [clang][WebAssembly] Initial support for reference type externref in clang (authored by pmatos). Repository: rG LLVM Github

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 493633. pmatos added a comment. Address last comment before landing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/ https://reviews.llvm.org/D122215 Files: clang/include/clang/AST/ASTContext.h

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Oops, it helps to actually accept it in Phab. :-D Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

aaron.ballman added a comment. Aside from a nit about comments, I don't have any further concerns. LGTM! Comment at: clang/include/clang/Basic/WebAssemblyReferenceTypes.def:16 +// +// - Name is the name of the builtin type. MangledName is the mangled name. +//

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

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

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos added a comment. I feel like the latest patch addresses all concerns until now. What do you think? Comment at: clang/include/clang/Basic/WebAssemblyReferenceTypes.def:16 +// +// - Name is the name of the builtin type. MangledName is the mangled name. +//

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 489520. pmatos marked 7 inline comments as done. pmatos added a comment. Rebase on main. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/ https://reviews.llvm.org/D122215 Files:

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 489005. pmatos added a comment. Rename test that still had tables in its name. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/ https://reviews.llvm.org/D122215 Files:

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 488999. pmatos added a comment. Fix tests after changing name mangling and error messages. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/ https://reviews.llvm.org/D122215 Files:

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 41. pmatos added a comment. Update mangling for Microsoft. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/ https://reviews.llvm.org/D122215 Files: clang/include/clang/AST/ASTContext.h

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:2479-2483 +#define WASM_REF_TYPE(InternalName, MangledName, Id, SingletonId, AS) \ + case BuiltinType::Id: \ +Out << "PA";

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

aaron.ballman added a subscriber: rnk. aaron.ballman added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:2479-2483 +#define WASM_REF_TYPE(InternalName, MangledName, Id, SingletonId, AS) \ + case BuiltinType::Id:

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 488648. pmatos added a comment. Now under the correct revision: @aaron.ballman Added support for externref mangling in Microsoft. This follows the pattern implemented by OpenCL types. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos marked an inline comment as done. pmatos added a comment. @aaron.ballman many thanks for the thorough reviews on the patches. Happy to see this moving in the right direction. Comment at: clang/lib/AST/MicrosoftMangle.cpp:2480-2481 #include

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 488634. pmatos marked an inline comment as done. pmatos added a comment. Address all comments except a couple of issues related to ABI and mangling. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos marked an inline comment as done. pmatos added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:806-807 + SingletonId = \ +

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

aaron.ballman added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:4012-4018 + if (Target->getTriple().isWasm() && Target->hasFeature("reference-types")) { +#define WASM_REF_TYPE(Name, MangledName, Id, SingletonId, AS) \ + if (BuiltinType::Id ==

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos added inline comments. Comment at: clang/lib/AST/MicrosoftMangle.cpp:2480-2481 #include "clang/Basic/RISCVVTypes.def" +#define WASM_TYPE(Name, Id, SingletonId) case BuiltinType::Id: +#include "clang/Basic/WebAssemblyReferenceTypes.def" case BuiltinType::ShortAccum:

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 488609. pmatos marked an inline comment as done. pmatos added a comment. Deal with Itanium Mangle and assert false outside Wasm triple. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos marked 5 inline comments as done. pmatos added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:4012-4018 + if (Target->getTriple().isWasm() && Target->hasFeature("reference-types")) { +#define WASM_REF_TYPE(Name, MangledName, Id, SingletonId, AS)

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 488598. pmatos added a comment. Address a couple of comments by @aaron.ballman Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/ https://reviews.llvm.org/D122215 Files:

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos marked 2 inline comments as done. pmatos added a comment. Addressed a few comments downstream. Will upload new patch next. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/ https://reviews.llvm.org/D122215

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

aaron.ballman added a subscriber: rjmccall. aaron.ballman added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:1480 + /// Return a WebAssembly externref type. + QualType getExternrefType() const; + `getWebAssemblyExternrefType` ?

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

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

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos added a comment. In D122215#4000612 , @MoritzS wrote: > In D122215#3998105 , @pmatos wrote: > >> In D122215#3991648 , @MoritzS >> wrote: >> >>> Thanks for the

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

MoritzS added a comment. In D122215#3998105 , @pmatos wrote: > In D122215#3991648 , @MoritzS wrote: > >> Thanks for the patch! I just tried it out and I think this enables many >> interesting use cases for

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos added a comment. In D122215#3991648 , @MoritzS wrote: > Thanks for the patch! I just tried it out and I think this enables many > interesting use cases for WebAssembly when using C/C++. > > Currently the lowering to wasm does not work when using

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 483203. pmatos added a comment. Force mem2reg on all optimization levels to avoid allocas. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/ https://reviews.llvm.org/D122215 Files:

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos added a comment. In D122215#3995105 , @pmatos wrote: > In D122215#3991648 , @MoritzS wrote: > >> Thanks for the patch! I just tried it out and I think this enables many >> interesting use cases for

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos added a comment. In D122215#3991648 , @MoritzS wrote: > Thanks for the patch! I just tried it out and I think this enables many > interesting use cases for WebAssembly when using C/C++. > > Currently the lowering to wasm does not work when using

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

MoritzS added a comment. Thanks for the patch! I just tried it out and I think this enables many interesting use cases for WebAssembly when using C/C++. Currently the lowering to wasm does not work when using externrefs when compiling without optimizations. Is that intended behavior? I have

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 479281. pmatos added a comment. Ready for review. RFC: https://discourse.llvm.org/t/rfc-webassembly-reference-types-in-clang/66939 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos planned changes to this revision. pmatos added a comment. Whole integration plan still in the works, so marking as planning changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/ https://reviews.llvm.org/D122215

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 470517. pmatos added a comment. Removed address space wasm_externref which is unnecessary with current design. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/ https://reviews.llvm.org/D122215 Files:

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos planned changes to this revision. pmatos added a comment. Current patch addresses most of the concerns - also passed through `./clang/tools/clang-format/git-clang-format`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 461518. pmatos added a comment. Address further concerns. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/ https://reviews.llvm.org/D122215 Files: clang/include/clang/AST/ASTContext.h

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos marked an inline comment as done. pmatos added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2258 +Width = 0; \ +Align = 8; /* ? */

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos marked an inline comment as done. pmatos added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:952-953 +12, // ptr64 +1, // wasm_var +10, // wasm_externref, }; aaron.ballman wrote: > How did you arrive at these

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 460706. pmatos marked 4 inline comments as done. pmatos added a comment. Further changes - more to come. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/ https://reviews.llvm.org/D122215 Files:

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos marked 10 inline comments as done. pmatos added inline comments. Comment at: clang/include/clang/Basic/AddressSpaces.h:59-60 + wasm_var, + wasm_externref, // This denotes the count of language-specific address spaces and also aaron.ballman wrote: >

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos marked 2 inline comments as done. pmatos added inline comments. Comment at: clang/include/clang/AST/Type.h:1972-1973 + /// Check if this is a WebAssembly Reference Type. + bool isWebAssemblyReferenceType() const; + bool isWebAssemblyExternrefType() const; ///

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos planned changes to this revision. pmatos added a comment. Still a few more changes are required. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/ https://reviews.llvm.org/D122215 ___

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 460696. pmatos marked an inline comment as done. pmatos added a comment. Fix a few of the issues mentioned in the revision comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/Type.h:1972-1973 + /// Check if this is a WebAssembly Reference Type. + bool isWebAssemblyReferenceType() const; + bool isWebAssemblyExternrefType() const; /// Determines if this is a sizeless type

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos marked 10 inline comments as done. pmatos added inline comments. Comment at: clang/include/clang/AST/Type.h:1972-1973 + /// Check if this is a WebAssembly Reference Type. + bool isWebAssemblyReferenceType() const; + bool isWebAssemblyExternrefType() const; ///

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

aaron.ballman requested changes to this revision. aaron.ballman added a comment. This revision now requires changes to proceed. Adding new types to the type system is quite invasive; was there an RFC for this you can point me to along with a design document? I have no idea how to review this

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

tlively added a comment. Very nice! This LGTM with all these small comments addressed. Sorry for the long delay in reviewing. Comment at: clang/include/clang/AST/ASTContext.h:1149 #include "clang/Basic/RISCVVTypes.def" +#define WASM_TYPE(Name, Id, SingletonId) CanQualType

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos added a comment. It would be great to get this landed. Can someone pls take a look at it? It's quite independent from the remainder of the work and self-contained. maybe @tlively ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

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

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 445493. pmatos added a comment. Remove some extraneous spaces I hadn't noticed before. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/ https://reviews.llvm.org/D122215 Files:

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 445490. pmatos added a comment. Updated patch over current main. Fixed all failint tests. Ready for review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/ https://reviews.llvm.org/D122215 Files:

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 438722. pmatos added a comment. rebase on top of D128282 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/ https://reviews.llvm.org/D122215 Files:

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 438622. pmatos added a comment. Incorporate @asb semantic restrictions patch. Generate test CHECK using update_cc_* script. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122215/new/

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

asb added a comment. I've left one minor comment. My other suggestion would be to absorb in the semantic restrictions work from here . Comment at: llvm/lib/IR/Type.cpp:307 +Type

[PATCH] D122215: [WebAssembly] Initial support for reference type externref in clang

pmatos updated this revision to Diff 437119. pmatos retitled this revision from "[WebAssembly] Initial support for reference types in clang" to "[WebAssembly] Initial support for reference type externref in clang". pmatos edited the summary of this revision. pmatos added a comment. Update