-
Notifications
You must be signed in to change notification settings - Fork 143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
1920x1080 output #9
Comments
The same problem is still present when compiled with 32a42dc |
With recent changes and the switch to WPELauncher the support for proper window sizing has been lost. I am working on fixing this today. If it's convenient for you, you could work around this issue by changing the hard-coded sizes in WPELauncher and WPEView: |
This has been fixed with the addition of the wpe-launcher package[1] and bumping the wpe version[2]. [1] Metrological/buildroot@7df6e9e |
…g LICM to miscompile https://bugs.webkit.org/show_bug.cgi?id=271435 rdar://124506508 Reviewed by Yusuke Suzuki. Consider the following example: ============================================================================================================ FIRST SLEEP (before performCFA) D@80:< 10:-> JSConstant(JS|PureNum|NeedsNegZero|NeedsNaNOrInfinity|UseAsOther, Final, Weak:Object: 0x13a0e8140 with butterfly 0x0(base=0xfffffffffffffff8) (Structure %AJ:Object), StructureID: 40640, bc#0, ExitValid) D@126:<!0:-> FilterGetByStatus(Check:Untyped:D@80, MustGen, (Simple, <id='uid:(x)', [0x300009ec0:[0x9ec0/40640, Object, (2/2, 0/0){x:0, toJSON:1}, NonArray, Proto:0x1180348d8]], [], offset = 0>, seenInJIT = true), W:SideState, bc#112, ExitValid) D@128:<!0:-> CheckStructure(Cell:D@80, MustGen, [%AJ:Object], R:JSCell_structureID, Exits, bc#112, ExitValid) D@133:<!0:-> FilterGetByStatus(Check:Untyped:D@80, MustGen, (Simple, <id='uid:(toJSON),cell:(String (atomic),8Bit:(1),length:(6): toJSON, StructureID: 16976)', [0x300009ec0:[0x9ec0/40640, Object, (2/2, 0/0){x:0, toJSON:1}, NonArray, Proto:0x1180348d8]], [], offset = 1>, seenInJIT = true), W:SideState, bc#118, ExitValid) D@136:< 4:-> GetByOffset(KnownCell:D@80, KnownCell:D@80, JS|PureNum|NeedsNaNOrInfinity|UseAsOther|ReallyWantsInt, BoolInt32, id6{toJSON}, 1, R:NamedProperties(6), bc#118, ExitValid) predicting BoolInt32 D@138:<!0:-> Check(Check:Int32:D@136, MustGen, Exits, bc#118, exit: bc#124, ExitValid) D@140:<!0:-> Branch(Boolean:D@35, MustGen, T:#9/w:10.000000, F:#12/w:10.000000, W:SideState, bc#124, ExitValid) D@4:< 1:-> GetButterfly(Cell:D@104, Storage|PureInt, R:JSObject_butterfly, bc#127, ExitValid) D@1:<!1:-> CheckInBounds(Int32:D@136, KnownInt32:D@151, JS|MustGen|PureInt, Int32, Exits, bc#127, ExitValid) D@143:< 3:-> GetByVal(KnownCell:D@104, Int32:Kill:D@136, Check:Untyped:Kill:D@4, Check:Untyped:Kill:D@1, JS|VarArgs|PureNum|NeedsNegZero|NeedsNaNOrInfinity|UseAsOther, StringIdent, Contiguous+OriginalCopyOnWriteArray+InBoundsSaneChain+AsIs+Read, R:Butterfly_publicLength,IndexedContiguousProperties, Exits, bc#127, ExitValid) predicting StringIdent %AJ:Object = 0x300009ec0:[0x9ec0/40640, Object, (2/2, 0/0){x:0, toJSON:1}, NonArray, Proto:0x1180348d8] Execution: AI GetByOffset D@136 AI says (BoolInt32, Int32: 0, none:StructuresAreClobbered) base: (Final, NonArray, [0x300009ec0:[0x9ec0/40640, Object, (2/2, 0/0){x:0, toJSON:1}, NonArray, Proto:0x1180348d8]], Object: 0x13a0e8140 with butterfly 0x0(base=0xfffffffffffffff8) (Structure 0x300009ec0:[0x9ec0/40640, Object, (2/2, 0/0){x:0, toJSON:1}, NonArray, Proto:0x1180348d8]), StructureID: 40640, 1:StructuresAreWatched) state StructuresAreWatched AI CheckInBounds D@1 AI says left Int32:D@136 is Int32: 0 SECOND SLEEP (after performCFA, before performConstantFolding) Note that the jsconstant has a structure transition at this point. D@80:< 10:-> JSConstant(JS|PureNum|NeedsNegZero|NeedsNaNOrInfinity|UseAsOther, Final, Weak:Object: 0x13a0e8140 with butterfly 0x8014002388(base=0x8014002380) (Structure %AR:Object), StructureID: 40976, bc#0, ExitValid) D@126:<!0:-> FilterGetByStatus(Check:Untyped:D@80, MustGen, (Simple, <id='uid:(x)', [0x300009ec0:[0x9ec0/40640, Object, (2/2, 0/0){toJSON:1, x:0}, NonArray, Proto:0x1180348d8]], [], offset = 0>, seenInJIT = true), W:SideState, bc#112, ExitValid) D@128:<!0:-> CheckStructure(Cell:D@80, MustGen, [%AR:Object], R:JSCell_structureID, Exits, bc#112, ExitValid) D@133:<!0:-> FilterGetByStatus(Check:Untyped:D@80, MustGen, (Simple, <id='uid:(toJSON),cell:(String (atomic),8Bit:(1),length:(6): toJSON, StructureID: 16976)', [0x300009ec0:[0x9ec0/40640, Object, (2/2, 0/0){toJSON:1, x:0}, NonArray, Proto:0x1180348d8]], [], offset = 1>, seenInJIT = true), W:SideState, bc#118, ExitValid) D@136:< 4:-> GetByOffset(KnownCell:D@80, KnownCell:D@80, JS|PureNum|NeedsNaNOrInfinity|UseAsOther|ReallyWantsInt, BoolInt32, id6{toJSON}, 1, R:NamedProperties(6), bc#118, ExitValid) predicting BoolInt32 D@4:< 1:-> GetButterfly(Cell:D@104, Storage|PureInt, R:JSObject_butterfly, bc#127, ExitValid) D@1:<!1:-> CheckInBounds(Int32:D@136, KnownInt32:D@151, JS|MustGen|PureInt, Int32, Exits, bc#127, ExitValid) D@143:< 3:-> GetByVal(KnownCell:D@104, Int32:Kill:D@136, Check:Untyped:Kill:D@4, Check:Untyped:Kill:D@1, JS|VarArgs|PureNum|NeedsNegZero|NeedsNaNOrInfinity|UseAsOther, StringIdent, Contiguous+OriginalCopyOnWriteArray+InBoundsSaneChain+AsIs+Read, R:Butterfly_publicLength,IndexedContiguousProperties, Exits, bc#127, ExitValid) predicting StringIdent %AR:Object = 0x300009ec0:[0x9ec0/40640, Object, (2/2, 0/0){toJSON:1, x:0}, NonArray, Proto:0x1180348d8] %B6:Object = 0x30000a010:[0xa010/40976, Object, (2/2, 1/4){y:64, toJSON:1, x:0}, NonArray, Proto:0x1180348d8, Leaf (Watched)] Execution: AI GetByOffset D@136 AI says (HeapTop, TOP, TOP, none:StructuresAreClobbered) base: (Final, NonArray, [0x300009ec0:[0x9ec0/40640, Object, (2/2, 0/0){toJSON:1, x:0}, NonArray, Proto:0x1180348d8]], Object: 0x13a0e8140 with butterfly 0x8014002388(base=0x8014002360) (Structure 0x30000a010:[0xa010/40976, Object, (2/2, 1/4){y:64, toJSON:1, x:0}, NonArray, Proto:0x1180348d8, Leaf (Watched)]), StructureID: 40976, 1:StructuresAreWatched) state StructuresAreWatched GetByOffset D@136 AI says (HeapTop, TOP, TOP, 1:StructuresAreWatched) CheckInBounds D@1 AI says left Int32:D@136 is Int32: 0 AI GetByOffset D@136 AI says (HeapTop, TOP, TOP, none:StructuresAreClobbered) base: (Final, NonArray, [0x300009ec0:[0x9ec0/40640, Object, (2/2, 0/0){toJSON:1, x:0}, NonArray, Proto:0x1180348d8]], Object: 0x13a0e8140 with butterfly 0x8014002388(base=0x8014002360) (Structure 0x30000a010:[0xa010/40976, Object, (2/2, 1/4){y:64, toJSON:1, x:0}, NonArray, Proto:0x1180348d8, Leaf (Watched)]), StructureID: 40976, 1:StructuresAreWatched) state StructuresAreWatched SLEEP DONE ============================================================================================================ The constant folding phase chooses to fold the CheckInBounds, but not the GetByOffset. At this point, this is still correct (although sub-optimal). 1) Why does AI disagree in these two places? The constant folding phase doesn't re-run AI. It runs it from top to bottom on certain blocks only. In this example, The CheckInBounds AI proof is read directly from the block, but the GetByOffset has its value computed. 1) Why can the JSConstant's structure change without triggering a watchpoint? The constant remains constant. We never used the fact that that it had a certain structure anywhere, our proofs stem from the fact that we have a CheckStructure. 1) Why does the re-run AI pass in performConstantFolding not predict the GetByOffset to be constant? The structure change causes GetPropertyConcurrently to fail to get the value concurrently. We must assume that it is always safe to produce a more conservative result in this phase. Note though that if the phase returned the same value as the first time around, that would still have been correct! The answer to this question didn't change, we just lost the ability to compute it. ============================================================================================================ Why this is a problem This is a classic example of a broad class of bugs affecting the JIT. Different passes can see different values as the mutator changes the object graph, even for the same pass. Normally this is fine, because the compiler is always narrowing its assumptions. Specifically, with each pass we assume more and more detailed things about the code, and guard against these assumptions being wrong either with watchpoints or runtime checks. In this example, we see that we CheckStructure. Then, as a result, we can elide nodes that are dominated by that check (like the GetByOffset or the CheckInBounds). As long as we never loosen that assumption again, we are fine. In this example, our CFA pass assumes that the GetByOffset is constant. The Constant Folding phase then assumes sometimes that it is constant, and sometimes that it is not. This puts us in opposition to another principle, that is the idea that we should always be able to answer any question asked of us conservatively and be safe. Up until this point, both of these ideas are holding true. Unfortunately, we also need LICM. LICM needs to run after many assumptions have already been made, and it dramatically loosens assumptions. In this example, LICM comes along and hoists the GetByVal(GetByOffset()) above the CheckStructure. If we had indeed constant folded the GetByOffset too, we would be fine to do. We should always be able to avoid constant folding safely. LICM should be able to hoist constant values safely. ============================================================================================================ How to fix this generally 1) If AI says something is constant, just make it constant then. This is the simplest solution, and should just work. This makes sure that what AI says is true, even if LICM moves stuff around. This would require some re-work of the AI phase though. 1) LCIM should see that this isn't safe to move The effects here are super specific. If LICM asked the question "If I move this, is this still safe to execute?" it would have answered "no" in this case (without the structure check). Of course, if we hadn't removed the CheckInBounds, the answer would be "yes," which is also fine. One could imagine that this analysis would be pretty difficult. 1) Always run the constant folder on each block. ``` // This method is evil - it causes a huge maintenance headache and there is a gross amount of // code devoted to it. It would be much nicer to just always run the constant folder on each // block. But, the last time we did it, it was a 1% SunSpider regression: // https://bugs.webkit.org/show_bug.cgi?id=133947 // So, we should probably keep this method. void setShouldTryConstantFolding(bool tryConstantFolding) { m_shouldTryConstantFolding = tryConstantFolding; } ``` This would fix the issue though, as a failure to prove something at any point in time would not permit that proof to be used later on. This patch chooses the third option. This appears to be perf-neutral on modern hardware on JS2/3 and SP2/3. * JSTests/stress/get-by-val-hoist-above-structure.js: Added. (opt): (createObjectOfS1): (createObjectOfS2): (main): * Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp: (JSC::DFG::ConstantFoldingPhase::foldConstants): Originally-landed-as: 272448.796@safari-7618-branch (8d5ba1eecf30). rdar://128088091 Canonical link: https://commits.webkit.org/278842@main
WPE compiled by buildroot rpi2_wpe_minimal_defconfig yesterday (at afd955b ) and a raspberry pi 2 running in 1080p resolution (hdm_group=1, hdmi_mode=16) results in a 1280x720 (720p) browser content "window", meaning that the page is displayed only in the top left corner of the screen, rest is black. Tried with our own material and the google front page with same results. A environment compiled on 2nd of September doesn't have this same issue.
The text was updated successfully, but these errors were encountered: