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
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/89655f00-a7b4-463d-b98f-3961cce576d9n%40googlegroups.com.
