Summary: | Drop s_exceptionInstructions static initializer | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||
Component: | bmalloc | Assignee: | Yusuke Suzuki <ysuzuki> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, darin, ews-watchlist, fpizlo, ggaren, joepeck, keith_miller, mark.lam, mitz, msaboff, saam, webkit-bug-importer, ysuzuki | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Joseph Pecoraro
2018-03-18 19:25:00 PDT
This doesn’t look to be related to Gigacage. __cpu_indicator_init supports __builtin_cpu_supports() and comes from the compiler (see e.g. <https://github.com/llvm-mirror/compiler-rt/blob/master/lib/builtins/cpu_model.c>). It may have ended up in JavaScriptCore because something used the builtin or the compiler thought it was needed. It doesn’t seem to be the case in TOT (although TOT has a static initializer for JSC::LLInt::Data::s_exceptionInstructions AFAICT). (In reply to mitz from comment #1) > This doesn’t look to be related to Gigacage. __cpu_indicator_init supports > __builtin_cpu_supports() and comes from the compiler (see e.g. > <https://github.com/llvm-mirror/compiler-rt/blob/master/lib/builtins/ > cpu_model.c>). It may have ended up in JavaScriptCore because something used > the builtin or the compiler thought it was needed. It doesn’t seem to be the > case in TOT (although TOT has a static initializer for > JSC::LLInt::Data::s_exceptionInstructions AFAICT). Nice catch. Created attachment 336032 [details]
Patch
(In reply to Yusuke Suzuki from comment #2) > (In reply to mitz from comment #1) > > This doesn’t look to be related to Gigacage. __cpu_indicator_init supports > > __builtin_cpu_supports() and comes from the compiler (see e.g. > > <https://github.com/llvm-mirror/compiler-rt/blob/master/lib/builtins/ > > cpu_model.c>). It may have ended up in JavaScriptCore because something used > > the builtin or the compiler thought it was needed. It doesn’t seem to be the > > case in TOT (although TOT has a static initializer for > > JSC::LLInt::Data::s_exceptionInstructions AFAICT). > > Nice catch. Actually, in production builds I still see the __cpu_indicator_init, so it would be nice to understand where it’s coming from (I don’t see any code actually using __builtin_cpu_supports()) and whether it can be eliminated. (In reply to mitz from comment #4) > (In reply to Yusuke Suzuki from comment #2) > > (In reply to mitz from comment #1) > > > This doesn’t look to be related to Gigacage. __cpu_indicator_init supports > > > __builtin_cpu_supports() and comes from the compiler (see e.g. > > > <https://github.com/llvm-mirror/compiler-rt/blob/master/lib/builtins/ > > > cpu_model.c>). It may have ended up in JavaScriptCore because something used > > > the builtin or the compiler thought it was needed. It doesn’t seem to be the > > > case in TOT (although TOT has a static initializer for > > > JSC::LLInt::Data::s_exceptionInstructions AFAICT). > > > > Nice catch. > > Actually, in production builds I still see the __cpu_indicator_init, so it > would be nice to understand where it’s coming from (I don’t see any code > actually using __builtin_cpu_supports()) and whether it can be eliminated. Oh, it still exists. OK, I've opened the bug for that. https://bugs.webkit.org/show_bug.cgi?id=183736 (In reply to mitz from comment #4) > Actually, in production builds I still see the __cpu_indicator_init, so it > would be nice to understand where it’s coming from (I don’t see any code > actually using __builtin_cpu_supports()) and whether it can be eliminated. __builtin_popcount? I remember that popcount requires relatively newer CPUs. Comment on attachment 336032 [details]
Patch
Thanks!
Comment on attachment 336032 [details] Patch Clearing flags on attachment: 336032 Committed r229740: <https://trac.webkit.org/changeset/229740> All reviewed patches have been landed. Closing bug. |