sbc100 added inline comments.
================ Comment at: clang/include/clang/Basic/AttrDocs.td:5608-5612 +Clang supports the ``__attribute__((wasm_async))`` +attribute for the WebAssembly target. This attribute may be attached to a +function definition, which indicates the function will be used with JavaScript +promise integration (JSPI). The attribute will cause the creation of a custom +section named "async" that contains each wasm_async function's index value. ---------------- sbc100 wrote: > sbc100 wrote: > > aaron.ballman wrote: > > > brendandahl wrote: > > > > aaron.ballman wrote: > > > > > aaron.ballman wrote: > > > > > > This could be my ignorance of web assembly showing, but the docs > > > > > > don't really help me understand when you'd want to use this > > > > > > attribute. Perhaps a link to what JSPI is and a code example would > > > > > > help a little bit? Or is this more of a low-level implementation > > > > > > detail kind of attribute where folks already know the domain? > > > > > Based on the documentation here, I'm wondering why the `annotate` > > > > > attribute doesn't suffice? That attribute lets you specify custom > > > > > information to associate with a declaration that then gets lowered > > > > > such that passes can do whatever they want with the info, which seems > > > > > to be a more generalized version of what this attribute is. > > > > > > > > > > (FWIW, I'm back to having basically no idea when you'd use this > > > > > attribute or what it would be used for, so my thoughts above might > > > > > make no sense.) > > > > I was considering that, but it would require more machinery in the wasm > > > > backend to store all the attribute values in the output. For now we > > > > only really need a flag associated with function. I think if we find > > > > more uses for the annotations in the future we could replace > > > > wasm_custom with it. > > > > I was considering that, but it would require more machinery in the wasm > > > > backend to store all the attribute values in the output. For now we > > > > only really need a flag associated with function. I think if we find > > > > more uses for the annotations in the future we could replace > > > > wasm_custom with it. > > > > > > More machinery in the backend is preferred to exposing a new attribute > > > that is this general-purpose; the backend is what needs this > > > functionality, the frontend basically does nothing with it. (I'm assuming > > > this is an implementation detail attribute and not something you expect > > > users to write. If I'm wrong about that, please let me know.) > > > Based on the documentation here, I'm wondering why the `annotate` > > > attribute doesn't suffice? > > > > I believe that was the original solution that was considered but Benden was > > told this was perhaps not an appropriate use of "annotate". Brenden can > > you elaborate on why? IIRC it was something like "the semantics of the > > program should not depend on annotate attributes but the attribute being > > added in this case is required for the program to run correctly".. is that > > about right? > > > > FWIW, I think using the existing `annotate` attribute would make a lot of > > sense... if we can get away with it. > > (I'm assuming this is an implementation detail attribute and not something > > you expect users to write. If I'm wrong about that, please let me know.) > > IIUC user would indeed be specifying these attributes. Kind of like they do > for "visibility" today. The initial attribute that motivated this change is > "async" which tells the host runtime that a certain function import or export > behaves in an async manor (See https://v8.dev/blog/jspi for more details). > > I was considering that, but it would require more machinery in the wasm > > backend to store all the attribute values in the output. For now we only > > really need a flag associated with function. I think if we find more uses > > for the annotations in the future we could replace wasm_custom with it. > > More machinery in the backend is preferred to exposing a new attribute that > is this general-purpose; the backend is what needs this functionality, the > frontend basically does nothing with it. (I'm assuming this is an > implementation detail attribute and not something you expect users to write. > If I'm wrong about that, please let me know.) I think perhaps there are two alternative implementations being proposed here, and I want to make sure we don't confuse them: 1. Use the existing `annotate` attribute. 2. Make an even more complex custom attribute that hold key=value pairs rather than the current CL which proposes simply boolean custom attributes (e.g. key=true). I think we would be happy with (1) if this usage is within scope. IIUC the thing that would take a lot more work in the backend (and more complex custom section design in the binary format) is (2). I don't think we need (2) today and we should probably wait until we have a use case before designed a more complex version of this attribute. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150803/new/ https://reviews.llvm.org/D150803 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
