Ah ok, thanks for that info.

That might also explain why I'm getting linker errors, eg 1 of about a 
dozen similar linker errors... any ideas how to patch this one?

cppgc_base.lib(marker.obj) : error LNK2019: unresolved external symbol 
"public: void __cdecl 
cppgc::internal::OldToNewRememberedSet::AddWeakCallback(struct 
cppgc::internal::MarkingWorklists::WeakCallbackItem)" 
(?AddWeakCallback@OldToNewRememberedSet@internal@cppgc@@QEAAXUWeakCallbackItem@MarkingWorklists@23@@Z)
 
referenced in function "public: void __cdecl 
cppgc::internal::MarkerBase::ProcessWeakness(void)" 
(?ProcessWeakness@MarkerBase@internal@cppgc@@QEAAXXZ)


On Monday, March 20, 2023 at 9:36:38 PM UTC+8 Jakob Kummerow wrote:

> 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/4ecabff1-ae07-47bc-9213-1cf4e6444f0dn%40googlegroups.com.

Reply via email to