540700 - We should insert LIR_regfence before npe, interrupt, rangeCheck

Status: VERIFIED FIXED

(Tamarin Graveyard :: Virtual Machine, defect, P2)

Description

[Steven Johnson]

Attachment: Patch

This will greatly reduce the likelihood that there will be (pointless) spill/restores around the branch sites.

Comment 1

[Edwin Smith]

As a separate patch we should look at whether a regfence would be beneficial for the cold paths of the stack overflow check (which does return) or the setjmp branch to the exception handler dispatcher.

Comment 2

[Edwin Smith]

Comment on attachment 422397 [details] [diff] [review]
Patch

A tweak like this deserves some comments explaining the win, or a comment referencing this bug, with some evidence of the win.  but it looks fine;  nice tweak, too.

Comment 3

[Steven Johnson]

Good point, I'll insert examples & commentary from our offlist discussion in comments here before landing.

Comment 4

[Steven Johnson]

This helps with situations like so... prior to patch, you could have code like

   ld61 = ld and2[992]
00133c5c13   mov ecx,992(esi)
00133c5c19   mov -108(ebp),ecx  <= spill ld61
                     RR eax(ld60) ecx(ld61) ebx(env) esi(and2)
   jt eq27 -> npe
00133c5c1c   test ecx,ecx
00133c5c1e   je    0x133c5d60
## merging registers (union) with existing edge
00133c5c24   mov ebx,-108(ebp)  <= restore ld61
                         RR eax(ld60) ebx(ld61) esi(and2) 

[npe]
0x133c5d60   opcode
                         RR ebx(whatever) 


i.e., since [npe] assumed contents of ebx, the branch to it had to do spill/restore.

Comment 5

[Steven Johnson]

Edwin also comments (from an offline discussion): "LIR_regfence was added specifically for this case; it causes register assignments in a cold branch (like npe, or like a side exit, or an uncommon path for an inlined operator) to not propagate upwards past the regfence where they would add register pressure to the hot path."

Comment 6

[Steven Johnson]

http://hg.mozilla.org/tamarin-redux/rev/9d3819da173d

Comment 7

[Edwin Smith]

Landed a while ago.

Comment 8

[Chris Peyer]

verified in argo: 9d3819da173d