Skip to content

IRC: Fix interf graph for exceptional path.#804

Merged
xclerc merged 3 commits intooxcaml:mainfrom
azewierzejew:irc_fix_exceptional_path
Sep 7, 2022
Merged

IRC: Fix interf graph for exceptional path.#804
xclerc merged 3 commits intooxcaml:mainfrom
azewierzejew:irc_fix_exceptional_path

Conversation

@azewierzejew
Copy link
Copy Markdown
Contributor

@azewierzejew azewierzejew commented Sep 1, 2022 •
edited
Loading

To create interf graph edges for exceptional branch there were edges add from every temporary variable to every physical register from Proc.destroyed_at_raise.

That is correct thing to do but the set of all live variables computed incorrectly.
The expected value is first_instr.before but without Proc.loc_exn_bucket, because that is "generated" by the exception.
But the value first_instr.across is currently taken.
If the first_instr is temporary/X = Proc.loc_exn_bucket then first_instr.across will be what we want because the temporary/X will be removed and Proc.loc_exn_bucket won't be added yet.

But in case where the first instruction isn't that (which can happen if the exception value is ignored and deadcode removes this move) then we would want exactly first_instr.before because we expect that it doesn't containt Proc.loc_exn_bucket as that's a preassigned register.
But first_instr.across compared to first_instr.before is missing all arguments of the instruction.
For that reason IRC doesn't mark those temporaries as colliding with Proc.loc_exn_bucket.

Then the registers can be left unspilled over an exception which isn't an allowed behaviour.

An example of this is

let foo x y = try x, y with _ -> raise Not_found

Upstream:

   0:	48 83 ec 08          	sub    rsp,0x8
   4:	49 8b be 88 00 00 00 	mov    rdi,QWORD PTR [r14+0x88]
   b:	48 89 3c 24          	mov    QWORD PTR [rsp],rdi
   f:	4c 8d 1d 3a 00 00 00 	lea    r11,[rip+0x3a]        # 50 <camlTest__foo_5+0x50>
  16:	41 53                	push   r11
  18:	41 ff 76 10          	push   QWORD PTR [r14+0x10]
  1c:	49 89 66 10          	mov    QWORD PTR [r14+0x10],rsp
  20:	49 83 ef 18          	sub    r15,0x18
  24:	4d 3b 7e 08          	cmp    r15,QWORD PTR [r14+0x8]
  28:	72 45                	jb     6f <camlTest__foo_5+0x6f>
  2a:	49 8d 7f 08          	lea    rdi,[r15+0x8]
  2e:	48 c7 47 f8 00 08 00 	mov    QWORD PTR [rdi-0x8],0x800
  35:	00 
  36:	48 89 07             	mov    QWORD PTR [rdi],rax
  39:	48 89 5f 08          	mov    QWORD PTR [rdi+0x8],rbx
  3d:	48 89 f8             	mov    rax,rdi
  40:	41 8f 46 10          	pop    QWORD PTR [r14+0x10]
  44:	48 83 c4 08          	add    rsp,0x8
  48:	48 83 c4 08          	add    rsp,0x8
  4c:	c3                   	ret    
  4d:	0f 1f 00             	nop    DWORD PTR [rax]
  50:	48 8b 04 24          	mov    rax,QWORD PTR [rsp]
  54:	49 89 86 88 00 00 00 	mov    QWORD PTR [r14+0x88],rax
  5b:	48 8b 05 00 00 00 00 	mov    rax,QWORD PTR [rip+0x0]        # 62 <camlTest__foo_5+0x62>
			5e: R_X86_64_GOTPCREL	caml_exn_Not_found-0x4
  62:	49 8b 66 10          	mov    rsp,QWORD PTR [r14+0x10]
  66:	41 8f 46 10          	pop    QWORD PTR [r14+0x10]
  6a:	41 5b                	pop    r11
  6c:	41 ff e3             	jmp    r11
  6f:	e8 00 00 00 00       	call   74 <camlTest__foo_5+0x74>
			70: R_X86_64_PLT32	caml_call_gc-0x4
  74:	eb b4                	jmp    2a <camlTest__foo_5+0x2a>
  76:	66 2e 0f 1f 84 00 00 	nop    WORD PTR cs:[rax+rax*1+0x0]

IRC:

0000000000000000 <camlTest__foo_5>:
   0:	48 83 ec 08          	sub    rsp,0x8
   4:	49 8b b6 88 00 00 00 	mov    rsi,QWORD PTR [r14+0x88]
   b:	4c 8d 1d 3a 00 00 00 	lea    r11,[rip+0x3a]        # 4c <camlTest__foo_5+0x4c>
  12:	41 53                	push   r11
  14:	41 ff 76 10          	push   QWORD PTR [r14+0x10]
  18:	49 89 66 10          	mov    QWORD PTR [r14+0x10],rsp
  1c:	49 83 ef 18          	sub    r15,0x18
  20:	4d 3b 7e 08          	cmp    r15,QWORD PTR [r14+0x8]
  24:	72 41                	jb     67 <camlTest__foo_5+0x67>
  26:	49 8d 7f 08          	lea    rdi,[r15+0x8]
  2a:	48 c7 47 f8 00 08 00 	mov    QWORD PTR [rdi-0x8],0x800
  31:	00 
  32:	48 89 07             	mov    QWORD PTR [rdi],rax
  35:	48 89 5f 08          	mov    QWORD PTR [rdi+0x8],rbx
  39:	48 89 f8             	mov    rax,rdi
  3c:	41 8f 46 10          	pop    QWORD PTR [r14+0x10]
  40:	48 83 c4 08          	add    rsp,0x8
  44:	48 83 c4 08          	add    rsp,0x8
  48:	c3                   	ret    
  49:	0f 1f 00             	nop    DWORD PTR [rax]
  4c:	49 89 b6 88 00 00 00 	mov    QWORD PTR [r14+0x88],rsi
  53:	48 8b 05 00 00 00 00 	mov    rax,QWORD PTR [rip+0x0]        # 5a <camlTest__foo_5+0x5a>
			56: R_X86_64_GOTPCREL	caml_exn_Not_found-0x4
  5a:	49 8b 66 10          	mov    rsp,QWORD PTR [r14+0x10]
  5e:	41 8f 46 10          	pop    QWORD PTR [r14+0x10]
  62:	41 5b                	pop    r11
  64:	41 ff e3             	jmp    r11
  67:	e8 00 00 00 00       	call   6c <camlTest__foo_5+0x6c>
			68: R_X86_64_PLT32	caml_call_gc-0x4
  6c:	eb b8                	jmp    26 <camlTest__foo_5+0x26>
  6e:	66 90                	xchg   ax,ax

The region pointer for IRC is stored in rsi while for Upstream it's spilled on stack.

In the handler the code generated by IRC reads the pointer from rsi but it could have been destroyed by the exception.

The Upstream code properly handles it by reload from stack slot which is persistent.

@gretay-js gretay-js added bug Something isn't working cfg backend labels Sep 1, 2022
@azewierzejew azewierzejew mentioned this pull request Sep 2, 2022
Merged
@xclerc xclerc merged commit bbb6b1a into oxcaml:main Sep 7, 2022
@azewierzejew azewierzejew deleted the irc_fix_exceptional_path branch September 7, 2022 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@xclerc xclerc xclerc approved these changes

@gretay-js gretay-js gretay-js approved these changes

Assignees

No one assigned

Labels

backend bug Something isn't working cfg

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@azewierzejew @xclerc @gretay-js