Bug 125581 - JS broken on ARMv6 because of dmb instruction
Summary: JS broken on ARMv6 because of dmb instruction
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 108645
  Show dependency treegraph
 
Reported: 2013-12-11 09:18 PST by Tomeu Vizoso
Modified: 2015-06-29 02:35 PDT (History)
3 users (show)

See Also:


Attachments
WIP patch (13.82 KB, patch)
2015-02-05 04:50 PST, Csaba Osztrogonác
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tomeu Vizoso 2013-12-11 09:18:37 PST
DMB was added in ARMv7, build is broken like this (sorry, no proper output handy):

/tmp/ccXgxs8q.s:4813: Error: selected processor does not support ARM mode `dmb sy'


tomeu`:  jbrianceau: regarding http://trac.webkit.org/changeset/159571 , I noticed that the implementation of memoryFence() fails to build on armv6 because dmb isn't available
tomeu`:  is that intentional?
jbrianceau:  tomeu`: no, not intentional
jbrianceau:  tomeu`: I didn't notice that because I have an armv7 target without thumb2 instruction set support
jbrianceau:  tomeu`: but if it fails to build, it should comes from LLINT and not http://trac.webkit.org/changeset/159571
tomeu`:  jbrianceau: ok, I was getting /tmp/ccXgxs8q.s:4813: Error: selected processor does not support ARM mode `dmb sy' and git log -Sdmb pointed to that commit
jbrianceau:  tomeu`: actually this commit would lead to runtime issues, not build
jbrianceau:  tomeu`: and your build issue should come from
jbrianceau:  tomeu`: this one : http://trac.webkit.org/changeset/159545
jbrianceau:  tomeu`: if you remove "$asm.puts "dmb sy"" from arm.rb file, it will build (but you'll have issues at runtime because of r159571)
tomeu`:  jbrianceau: ok, I know close to nothing about ARM assembler, but I have been told that on ARMv6 that can safely be a nop or at most a mcr?
jbrianceau:  tomeu`: ok. If a nop is fine, then you might want to replace "dmb sy" by "nop" in arm.rb file, and also
jbrianceau:  tomeu`: replace m_assembler.dmbSY() by m_assembler.nop() in MacroAssemblerARM.h file
jbrianceau:  tomeu`: it should build and run properly if what you've been told is ok :)
tomeu`:  jbrianceau: oh, I solved my particular issue by rebasing my branch before the commit that introduced the fence :)
tomeu`:  was just saying for the future
tomeu`:  will open a bug with our conversation
jbrianceau:  tomeu`: allright
Comment 1 Csaba Osztrogonác 2014-09-29 08:44:58 PDT
FYI: The "fancy fencing" isn't needed anymore after https://trac.webkit.org/changeset/159798. So it's safe to remove this code. But I'm not sure if
JavaScriptCore maintainers would like to keep it in trunk for future use.
Comment 2 Csaba Osztrogonác 2015-02-05 04:50:17 PST
Created attachment 246097 [details]
WIP patch

remove unused fencing mechanism, WIP patch, not tested anywhere