Okay, that makes sense. In Chromium you just need a blank line but I'll add an explanatory comment as well.
Any thoughts on intrin.h? It works in my debug builds in the context of Chromium, but not on the bots. I think intrin.h must be different there. I can keep exploring if nobody knows off the top of their head. On Fri, Jul 23, 2021 at 8:34 AM Dan Elphick <delph...@chromium.org> wrote: > git cl format won't reorder headers separated by a comment (possibly needs > a newline as well but that looks more natural anyway). > > On Fri, 23 Jul 2021 at 16:27, 'Bruce Dawson' via v8-dev < > v8-dev@googlegroups.com> wrote: > >> I'll try std::aligned_storage. It sounds like that will be perfect. I can >> just drop it in to replace the existing declarations that I created. >> >> You can see the 99% working CL at crrev.com/c//3042215. The only problem >> is that something doesn't work with intrin.h on cppgc. I include intrin.h >> and then get an error saying: >> >> note: include the header <intrin.h> or explicitly provide a declaration >> for '__readfsdword' >> >> >> I'm also hitting problems where "git cl format" reorders my #includes in >> a way that breaks compilation and I don't know how to prevent that. If I >> can get suggestions for those two issues then I can finish this off. >> >> On Fri, Jul 23, 2021 at 7:44 AM Leszek Swirski <lesz...@chromium.org> >> wrote: >> >>> Or by using std::aligned_storage >>> <https://en.cppreference.com/w/cpp/types/aligned_storage> + some static >>> asserts that sizes match, if we need to actually allocate storage for these >>> types... >>> >>> On Fri, Jul 23, 2021 at 4:40 PM Leszek Swirski <lesz...@chromium.org> >>> wrote: >>> >>>> I don't suppose we could just type erase via void*? That should at >>>> least be UB-clean. I do think we, in reality, have also given up on strict >>>> aliasing, but we nevertheless do try to go via bitcast where possible. I >>>> kicked off some UBSan bots on your CL, let's see what they say. >>>> >>>> On Thu, Jul 22, 2021 at 9:43 PM 'Bruce Dawson' via v8-dev < >>>> v8-dev@googlegroups.com> wrote: >>>> >>>>> The proxy-type issue is indeed the one wart in this project. I know >>>>> that Chrome (at least on Windows, and probably everywhere) has decided not >>>>> to enforce strict aliasing, but V8 may not have given up on that. >>>>> >>>>> The proxy types are rarely if ever read from or written to using the >>>>> proxy type, but I'm not sure if that makes them legal or if that just >>>>> means >>>>> that every reference is UB - I suspect every reference is UB. And, I can't >>>>> guarantee that they are only read/written using the real type - object >>>>> copying would necessarily copy the bits using the proxy type. >>>>> >>>>> I don't think that windows.h can be removed without using this >>>>> technique (not unless every one of these becomes a pointer to an allocated >>>>> object). In the Chromium code-base I decided that the ugliness of the >>>>> aliasing was less than the ugliness of windows-h-everywhere. V8 may, of >>>>> course, come to a different conclusion. >>>>> >>>>> I updated my CL which uses the proxy technique on a couple of types so >>>>> that it will build. That will let us see what the UBSan bots think of it. >>>>> Is the regular dry run sufficient? >>>>> >>>>> On Thu, Jul 22, 2021 at 11:55 AM Leszek Swirski <lesz...@chromium.org> >>>>> wrote: >>>>> >>>>>> Hi Bruce, >>>>>> >>>>>> Thanks for looking into this, I'd be happy to do code reviews and >>>>>> give advice if you're ok with the EMEA timezone delay - otherwise we've a >>>>>> couple of people in PST too. >>>>>> >>>>>> One question about the proxy types: are we at risk of triggering >>>>>> undefined behaviour by reinterpret casting between the proxy types and >>>>>> the >>>>>> Windows types? We've gone to some lengths to be UBSan clean and I >>>>>> wouldn't >>>>>> want to regress that. >>>>>> >>>>>> Cheers, >>>>>> Leszek >>>>>> >>>>>> >>>>>> On Thu, 22 Jul 2021, 19:57 'Bruce Dawson' via v8-dev, < >>>>>> v8-dev@googlegroups.com> wrote: >>>>>> >>>>>>> To make this more concrete I created a V8 CL >>>>>>> <https://chromium-review.googlesource.com/c/v8/v8/+/3042215> that >>>>>>> demonstrates what is involved, >>>>>>> using obj/v8/v8_libbase/condition-variable.obj as the test case. That >>>>>>> file >>>>>>> builds. Other files do not. >>>>>>> >>>>>>> Some of the files are just hacked into compliance, but it gives a >>>>>>> sense of how it would work. >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Thu, Jul 22, 2021 at 10:23 AM Bruce Dawson < >>>>>>> brucedaw...@google.com> wrote: >>>>>>> >>>>>>>> I'm a Chrome developer and I've been working on reducing how >>>>>>>> frequently windows.h is included in source files when building Chrome >>>>>>>> for >>>>>>>> Windows. It used to be (2018) that more than 78% of Chrome's >>>>>>>> translation >>>>>>>> units included windows.h When I picked up this project again in June >>>>>>>> this >>>>>>>> had been reduced and about half of Chrome's translation units pulled in >>>>>>>> windows.h. Since June I have reduced this to less than 22%. About 9% >>>>>>>> of the >>>>>>>> remaining uses of windows.h (2% of source files used when building >>>>>>>> Chrome) >>>>>>>> are in v8 - 761 translation units. >>>>>>>> >>>>>>>> The advantages of reducing usage of windows.h are (very slightly) >>>>>>>> reduced compile times, reduced namespace pollution (no more #define >>>>>>>> DrawText DrawTextW), reduced warning-flag manipulation, etc. Mostly >>>>>>>> it's >>>>>>>> about avoiding those macro definitions. >>>>>>>> >>>>>>>> So... >>>>>>>> >>>>>>>> I'm looking at getting v8 to use windows.h less. The techniques are >>>>>>>> well understood and proven and are best summarized in the contents of >>>>>>>> windows_types.h >>>>>>>> <https://source.chromium.org/chromium/chromium/src/+/main:base/win/windows_types.h;l=9?q=windows_types.h&sq=>. >>>>>>>> This creates the minimal set of typedefs and defines needed to compile >>>>>>>> Chrome's header files and portable code. In a few cases >>>>>>>> (CONDITION_VARIABLE, for instance) it is necessary to define Chrome >>>>>>>> proxy's >>>>>>>> for Windows types, and cast between them when calling Windows >>>>>>>> functions >>>>>>>> <https://source.chromium.org/chromium/chromium/src/+/main:base/win/windows_types.h;l=299?q=windows_types.h> >>>>>>>> . >>>>>>>> >>>>>>>> This is a medium sized project that will require landing lots of >>>>>>>> small CLs. I've done enough investigation to get a sense of the scope. >>>>>>>> I'll >>>>>>>> need to create V8 proxy-types for CONDITION_VARIABLE, SRWLOCK, and >>>>>>>> CRITICAL_SECTION, create typedefs for HANDLE (this is easy - there's no >>>>>>>> casting required), and explicitly include windows.h in a few source >>>>>>>> files >>>>>>>> that actually need it. And I'll need to fix whatever other issues pop >>>>>>>> up, >>>>>>>> but I'm not expecting anything worse than what I've seen before. I >>>>>>>> think >>>>>>>> that the vast majority of v8 files will be able to compile without >>>>>>>> windows.h. >>>>>>>> >>>>>>>> The overall project is tracked by crbug.com/796644 and the bugs >>>>>>>> which block it. >>>>>>>> >>>>>>>> Is this a project that V8 would support? If so, can I get a >>>>>>>> volunteer/sponsor who I can work with for advice and some code reviews? >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Bruce Dawson, he/him >>>>>>> >>>>>>> -- >>>>>>> -- >>>>>>> v8-dev mailing list >>>>>>> v8-dev@googlegroups.com >>>>>>> http://groups.google.com/group/v8-dev >>>>>>> --- >>>>>>> You received this message because you are subscribed to the Google >>>>>>> Groups "v8-dev" group. >>>>>>> To unsubscribe from this group and stop receiving emails from it, >>>>>>> send an email to v8-dev+unsubscr...@googlegroups.com. >>>>>>> To view this discussion on the web visit >>>>>>> https://groups.google.com/d/msgid/v8-dev/CAE5mQiMF_Yx4cvVKtfhw1bGxqdKCx0U7%3DEz%2BMqGm%2BO%2BBG47U7w%40mail.gmail.com >>>>>>> <https://groups.google.com/d/msgid/v8-dev/CAE5mQiMF_Yx4cvVKtfhw1bGxqdKCx0U7%3DEz%2BMqGm%2BO%2BBG47U7w%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>>>> . >>>>>>> >>>>>> -- >>>>>> -- >>>>>> v8-dev mailing list >>>>>> v8-dev@googlegroups.com >>>>>> http://groups.google.com/group/v8-dev >>>>>> --- >>>>>> You received this message because you are subscribed to a topic in >>>>>> the Google Groups "v8-dev" group. >>>>>> To unsubscribe from this topic, visit >>>>>> https://groups.google.com/d/topic/v8-dev/MbE-A6QBOCM/unsubscribe. >>>>>> To unsubscribe from this group and all its topics, send an email to >>>>>> v8-dev+unsubscr...@googlegroups.com. >>>>>> To view this discussion on the web visit >>>>>> https://groups.google.com/d/msgid/v8-dev/CAGRskv-kw1aFXyK%2BQwmOffvh3TVZvWgHHw6MBpoRMeGhGTJ2Hg%40mail.gmail.com >>>>>> <https://groups.google.com/d/msgid/v8-dev/CAGRskv-kw1aFXyK%2BQwmOffvh3TVZvWgHHw6MBpoRMeGhGTJ2Hg%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>>> . >>>>>> >>>>> >>>>> >>>>> -- >>>>> Bruce Dawson, he/him >>>>> >>>>> -- >>>>> -- >>>>> v8-dev mailing list >>>>> v8-dev@googlegroups.com >>>>> http://groups.google.com/group/v8-dev >>>>> --- >>>>> You received this message because you are subscribed to the Google >>>>> Groups "v8-dev" group. >>>>> To unsubscribe from this group and stop receiving emails from it, send >>>>> an email to v8-dev+unsubscr...@googlegroups.com. >>>>> To view this discussion on the web visit >>>>> https://groups.google.com/d/msgid/v8-dev/CAE5mQiMsatJLmHW9Aufq5EVob3DYQCYa8h_xs7-bfVBLXngOjA%40mail.gmail.com >>>>> <https://groups.google.com/d/msgid/v8-dev/CAE5mQiMsatJLmHW9Aufq5EVob3DYQCYa8h_xs7-bfVBLXngOjA%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>> . >>>>> >>>> -- >>> -- >>> v8-dev mailing list >>> v8-dev@googlegroups.com >>> http://groups.google.com/group/v8-dev >>> --- >>> You received this message because you are subscribed to a topic in the >>> Google Groups "v8-dev" group. >>> To unsubscribe from this topic, visit >>> https://groups.google.com/d/topic/v8-dev/MbE-A6QBOCM/unsubscribe. >>> To unsubscribe from this group and all its topics, send an email to >>> v8-dev+unsubscr...@googlegroups.com. >>> To view this discussion on the web visit >>> https://groups.google.com/d/msgid/v8-dev/CAGRskv9h3KKW59O4VswHZhCkSdPg-wx6JLy0Mc7jkDw640woWg%40mail.gmail.com >>> <https://groups.google.com/d/msgid/v8-dev/CAGRskv9h3KKW59O4VswHZhCkSdPg-wx6JLy0Mc7jkDw640woWg%40mail.gmail.com?utm_medium=email&utm_source=footer> >>> . >>> >> >> >> -- >> Bruce Dawson, he/him >> >> -- >> -- >> v8-dev mailing list >> v8-dev@googlegroups.com >> http://groups.google.com/group/v8-dev >> --- >> You received this message because you are subscribed to the Google Groups >> "v8-dev" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to v8-dev+unsubscr...@googlegroups.com. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/v8-dev/CAE5mQiPtfWwttimwqxoMVkT0aO8q-hjSLCeZgoFDY0jjO62STg%40mail.gmail.com >> <https://groups.google.com/d/msgid/v8-dev/CAE5mQiPtfWwttimwqxoMVkT0aO8q-hjSLCeZgoFDY0jjO62STg%40mail.gmail.com?utm_medium=email&utm_source=footer> >> . >> > -- > -- > v8-dev mailing list > v8-dev@googlegroups.com > http://groups.google.com/group/v8-dev > --- > You received this message because you are subscribed to a topic in the > Google Groups "v8-dev" group. > To unsubscribe from this topic, visit > https://groups.google.com/d/topic/v8-dev/MbE-A6QBOCM/unsubscribe. > To unsubscribe from this group and all its topics, send an email to > v8-dev+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/v8-dev/CALH_77tLNROkW7dDzj%3Dg4HK%2BMk%3DrZKRgYP%3Dq%3DTWtkMaeb1-Sow%40mail.gmail.com > <https://groups.google.com/d/msgid/v8-dev/CALH_77tLNROkW7dDzj%3Dg4HK%2BMk%3DrZKRgYP%3Dq%3DTWtkMaeb1-Sow%40mail.gmail.com?utm_medium=email&utm_source=footer> > . > -- Bruce Dawson, he/him -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are subscribed to the Google Groups "v8-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/CAE5mQiNDjQjWJTnAwpYpvRB0UPN875a3CLYdqXDPdGxSKQgXRQ%40mail.gmail.com.
