Like other non-Clang compilers, MSVC is largely community-supported: we have a CI bot for it that we're keeping green, but it only tests one configuration, and other configurations (e.g. other MSVC versions, or other GN args) tend to break from time to time. Please feel free to submit a patch <https://v8.dev/docs/contribute> to fix MSVC-related issues.
On Fri, Mar 17, 2023 at 5:13 AM Paul Harris <[email protected]> wrote: > Sorry, that patch won't work as you can't put ({ }) code just anywhere in > MSVC (gcc likes it). > > I gave up trying to please msvc, and in the end moved forward with this > patch... ie using an inline function > I know it'll probably mess with the crash location of chromium dumps, but > it was time to cut losses... > > --- a/src/base/immediate-crash.h > +++ b/src/base/immediate-crash.h > @@ -154,9 +154,9 @@ > > #else > > // This is supporting build with MSVC where there is no > __builtin_unreachable(). > -#define IMMEDIATE_CRASH() WRAPPED_TRAP_SEQUENCE_() > +inline void IMMEDIATE_CRASH() { WRAPPED_TRAP_SEQUENCE_(); } > > #endif // defined(__clang__) || defined(COMPILER_GCC) > > #endif // V8_BASE_IMMEDIATE_CRASH_H_ > > > On Friday, March 17, 2023 at 11:13:05 AM UTC+8 Paul Harris wrote: > >> Hello all, >> >> I'm building 11.0.226.19 with VS 17.5.1 >> ie MSVC >> ie cl.exe 19.35.32215 >> >> tldr is: >> ``` >> --- immediate-crash.h.orig 2023-03-17 10:59:27.251900900 +0800 >> +++ immediate-crash.h 2023-03-17 10:59:37.427856200 +0800 >> @@ -144,19 +144,22 @@ >> >> #if defined(__clang__) || V8_CC_GCC >> >> // __builtin_unreachable() hints to the compiler that this is noreturn >> and can >> // be packed in the function epilogue. >> #define IMMEDIATE_CRASH() \ >> ({ \ >> WRAPPED_TRAP_SEQUENCE_(); \ >> __builtin_unreachable(); \ >> }) >> >> #else >> >> // This is supporting build with MSVC where there is no >> __builtin_unreachable(). >> -#define IMMEDIATE_CRASH() WRAPPED_TRAP_SEQUENCE_() >> +#define IMMEDIATE_CRASH() \ >> + ({ \ >> + WRAPPED_TRAP_SEQUENCE_(); \ >> + }) >> >> #endif // defined(__clang__) || defined(COMPILER_GCC) >> >> #endif // V8_BASE_IMMEDIATE_CRASH_H_ >> ``` >> >> I'm not sure why MSVC didn't get the ({ brackets }) >> nor do I know why noone else has seen this yet. >> >> The problem started with this change: >> https://chromium-review.googlesource.com/c/v8/v8/+/3899298/95 >> >> In particular, the new macro LABEL_BLOCK in line 59 here: >> >> https://chromium-review.googlesource.com/c/v8/v8/+/3899298/95/src/compiler/turboshaft/assembler.h >> >> >> When building with is_official_build=true, the following happens: >> >> src/compiler/turboshaft/machine-optimization-reducer.h : 1669 >> OpIndex ReducePhi(base::Vector<const OpIndex> inputs, >> RegisterRepresentation rep) { >> LABEL_BLOCK(no_change) { return Next::ReducePhi(inputs, rep); } >> >> >> LABEL_BLOCK line becomes: >> for (; false; UNREACHABLE()) no_change: { return Next::ReducePhi(inputs, >> rep); >> } >> >> focus on the for loop, that becomes: >> for (; false; FATAL(message)) >> becomes: >> for (; false; IMMEDIATE_CRASH()) >> becomes: >> for (; false; WRAPPED_TRAP_SEQUENCE_()) >> becomes: >> for (; false; TRAP_SEQUENCE_()) >> becomes: >> for (; false; do { TRAP_SEQUENCE1_(); TRAP_SEQUENCE2_(); } while (false)) >> becomes: >> for (; false; do { __debugbreak(); ; } while (false)) >> Which fails on all compilers (including gcc) thanks to the do following >> the ; >> >> This would have compiled if it had some extra brackets in there: >> for (; false; ({ do { __debugbreak(); ; } while (false) }) ) >> >> What can we do about putting in this tiny patch? >> And, is noone at google using MSVC ? >> >> cheers, >> Paul >> >> -- > -- -- v8-dev mailing list [email protected] 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 [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/v8-dev/CAKSzg3S%3DcCA8G3zNY-vfHLnKyP1sWpBghfsgthcjHdTH%2B5ansQ%40mail.gmail.com.
