Bug 125216 – Do not emit #if-#endif guards for caluclation the number of StructOffset and Sizeof nodes.

• Status: RESOLVED
• Resolution: WONTFIX
• Component: JavaScriptCore

[László Langó]

Do not emit #if-#endif guards for calculation the number of StructOffset and Sizeof nodes.

[László Langó]

Created attachment 218395 [details]
Patch

[Filip Pizlo]

Comment on attachment 218395 [details]
Patch

Why do you think this is right?  Note that this code is for MachO fat binary support so please test a fat binary build on Mac.

[László Langó]

(In reply to comment #2)
> (From update of attachment 218395 [details])
> Why do you think this is right?  Note that this code is for MachO fat binary support so please test a fat binary build on Mac.

Ok, if you can tell me how to make a Mach0 fat binary, then i'll check this on Mac too. But this should be Ok, bacause this change is only remove unused defines from LLIntDesiredOffsets.h. In the beginning of this file the empty defines are useless, they are there because of the wrong function call, when the cloop.rb calculates the length value:

#if !OFFLINE_ASM_X86 && !OFFLINE_ASM_ARMv7_TRADITIONAL && !OFFLINE_ASM_ARMv7 && OFFLINE_ASM_ASSERT_ENABLED && OFFLINE_ASM_VALUE_PROFILER && OFFLINE_ASM_BIG_ENDIAN && OFFLINE_ASM_ALWAYS_ALLOCATE_SLOW && !OFFLINE_ASM_MIPS && OFFLINE_ASM_ARMv7s && OFFLINE_ASM_EXECUTION_TRACING && OFFLINE_ASM_JSVALUE64 && !OFFLINE_ASM_ARM64 && !OFFLINE_ASM_X86_64 && OFFLINE_ASM_JIT_ENABLED && !OFFLINE_ASM_SH4 && OFFLINE_ASM_C_LOOP && OFFLINE_ASM_JAVASCRIPT_DEBUGGER && !OFFLINE_ASM_ARM
#endif
#if !OFFLINE_ASM_X86 && !OFFLINE_ASM_ARMv7_TRADITIONAL && !OFFLINE_ASM_ARMv7 && OFFLINE_ASM_ASSERT_ENABLED && OFFLINE_ASM_VALUE_PROFILER && OFFLINE_ASM_BIG_ENDIAN && OFFLINE_ASM_ALWAYS_ALLOCATE_SLOW && !OFFLINE_ASM_MIPS && OFFLINE_ASM_ARMv7s && OFFLINE_ASM_EXECUTION_TRACING && OFFLINE_ASM_JSVALUE64 && !OFFLINE_ASM_ARM64 && !OFFLINE_ASM_X86_64 && OFFLINE_ASM_JIT_ENABLED && !OFFLINE_ASM_SH4 && OFFLINE_ASM_C_LOOP && !OFFLINE_ASM_JAVASCRIPT_DEBUGGER && !OFFLINE_ASM_ARM
#endif

...

#if !OFFLINE_ASM_X86 && !OFFLINE_ASM_ARMv7_TRADITIONAL && !OFFLINE_ASM_ARMv7 && !OFFLINE_ASM_ASSERT_ENABLED && !OFFLINE_ASM_VALUE_PROFILER && !OFFLINE_ASM_BIG_ENDIAN && !OFFLINE_ASM_ALWAYS_ALLOCATE_SLOW && !OFFLINE_ASM_MIPS && !OFFLINE_ASM_ARMv7s && !OFFLINE_ASM_EXECUTION_TRACING && !OFFLINE_ASM_JSVALUE64 && !OFFLINE_ASM_ARM64 && !OFFLINE_ASM_X86_64 && !OFFLINE_ASM_JIT_ENABLED && !OFFLINE_ASM_SH4 && !OFFLINE_ASM_C_LOOP && !OFFLINE_ASM_JAVASCRIPT_DEBUGGER && !OFFLINE_ASM_ARM
#endif
static const unsigned extractorTable[1139200] = {

For the length we need to iterate through them, but we don't need to emit the defines. And the extra is that we can save ~1MB space with this change.

[Filip Pizlo]

(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 218395 [details] [details])
> > Why do you think this is right?  Note that this code is for MachO fat binary support so please test a fat binary build on Mac.
> 
> Ok, if you can tell me how to make a Mach0 fat binary, then i'll check this on Mac too. But this should be Ok, bacause this change is only remove unused defines from LLIntDesiredOffsets.h. In the beginning of this file the empty defines are useless, they are there because of the wrong function call, when the cloop.rb calculates the length value:
> 
> #if !OFFLINE_ASM_X86 && !OFFLINE_ASM_ARMv7_TRADITIONAL && !OFFLINE_ASM_ARMv7 && OFFLINE_ASM_ASSERT_ENABLED && OFFLINE_ASM_VALUE_PROFILER && OFFLINE_ASM_BIG_ENDIAN && OFFLINE_ASM_ALWAYS_ALLOCATE_SLOW && !OFFLINE_ASM_MIPS && OFFLINE_ASM_ARMv7s && OFFLINE_ASM_EXECUTION_TRACING && OFFLINE_ASM_JSVALUE64 && !OFFLINE_ASM_ARM64 && !OFFLINE_ASM_X86_64 && OFFLINE_ASM_JIT_ENABLED && !OFFLINE_ASM_SH4 && OFFLINE_ASM_C_LOOP && OFFLINE_ASM_JAVASCRIPT_DEBUGGER && !OFFLINE_ASM_ARM
> #endif
> #if !OFFLINE_ASM_X86 && !OFFLINE_ASM_ARMv7_TRADITIONAL && !OFFLINE_ASM_ARMv7 && OFFLINE_ASM_ASSERT_ENABLED && OFFLINE_ASM_VALUE_PROFILER && OFFLINE_ASM_BIG_ENDIAN && OFFLINE_ASM_ALWAYS_ALLOCATE_SLOW && !OFFLINE_ASM_MIPS && OFFLINE_ASM_ARMv7s && OFFLINE_ASM_EXECUTION_TRACING && OFFLINE_ASM_JSVALUE64 && !OFFLINE_ASM_ARM64 && !OFFLINE_ASM_X86_64 && OFFLINE_ASM_JIT_ENABLED && !OFFLINE_ASM_SH4 && OFFLINE_ASM_C_LOOP && !OFFLINE_ASM_JAVASCRIPT_DEBUGGER && !OFFLINE_ASM_ARM
> #endif
> 
> ...
> 
> #if !OFFLINE_ASM_X86 && !OFFLINE_ASM_ARMv7_TRADITIONAL && !OFFLINE_ASM_ARMv7 && !OFFLINE_ASM_ASSERT_ENABLED && !OFFLINE_ASM_VALUE_PROFILER && !OFFLINE_ASM_BIG_ENDIAN && !OFFLINE_ASM_ALWAYS_ALLOCATE_SLOW && !OFFLINE_ASM_MIPS && !OFFLINE_ASM_ARMv7s && !OFFLINE_ASM_EXECUTION_TRACING && !OFFLINE_ASM_JSVALUE64 && !OFFLINE_ASM_ARM64 && !OFFLINE_ASM_X86_64 && !OFFLINE_ASM_JIT_ENABLED && !OFFLINE_ASM_SH4 && !OFFLINE_ASM_C_LOOP && !OFFLINE_ASM_JAVASCRIPT_DEBUGGER && !OFFLINE_ASM_ARM
> #endif
> static const unsigned extractorTable[1139200] = {
> 
> For the length we need to iterate through them, but we don't need to emit the defines. And the extra is that we can save ~1MB space with this change.

This calls the right method because that's how we handle fat binaries. Please don't break this.