Bug 23472 – scope(sucess) generate exception handling code.

Status
NEW
Severity
normal
Priority
P3
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2022-11-09T14:29:10Z
Last change time
2024-12-13T19:25:35Z
Assigned to
No Owner
Creator
deadalnix
Moved to GitHub: dmd#20182 →

Comments

Comment #0 by deadalnix — 2022-11-09T14:29:10Z
See sample code: char inc(string s, ref int i) { scope(success) i++; return s[i]; } void main() { int i; inc("string", i); } The codegen using LDC is: char example.inc(immutable(char)[], ref int): push rbx mov rbx, rdx movsxd rcx, dword ptr [rdx] cmp rdi, rcx jbe .LBB0_1 mov al, byte ptr [rsi + rcx] add ecx, 1 mov dword ptr [rbx], ecx pop rbx ret .LBB0_1: mov r8, rdi lea rsi, [rip + .L.str] mov edi, 14 mov edx, 3 call _d_arraybounds_index@PLT cmp edx, 1 jne .LBB0_7 mov rdi, rax call _d_eh_enter_catch@PLT mov rdi, rax call _d_throw_exception@PLT .LBB0_7: add dword ptr [rbx], 1 mov rdi, rax call _Unwind_Resume@PLT mov rdi, rax call _Unwind_Resume@PLT It contains exception handling code, which shouldn't be required for a scope(success) statement, and in fact, we can see that the exception is caught and rethrown immediately. Checking with GDC and DMD shows that they also contain exception handling code, so I assume this is being generated by the frontend.
Comment #1 by kinke — 2022-11-09T16:18:57Z
The frontend lowers this to: ``` char inc(string s, ref int i) { bool __os2 = false; try { try { return s[cast(ulong)i]; } catch(Throwable __o3) { __os2 = true; throw __o3; } } finally if (!__os2) i++; } ``` So the increment is to be skipped if the bounds check fails. Using -release etc. allows the LLVM optimizer to get rid of all EH boilerplate.
Comment #2 by deadalnix — 2022-11-09T16:39:47Z
`__os2` can only be false if no exception was thrown. There is no reason to emit any exception handling code in such function, even in the instances where the bound check might fail. In fact, the whole point of scope success is "run this when the scope is exited using regular control flow rather than exception handling", so there is no reason to emit any exception handling code ever when dealing with scope(success).
Comment #3 by kinke — 2022-11-09T18:57:27Z
> so there is no reason to emit any exception handling code ever when > dealing with scope(success). That only holds for trivial code like your example. With multiple return statements like char inc(string s, ref int i) { scope(success) i++; if (s == null) return 0; if (s.length > 256) return 1; return s[i]; } the general frontend lowering does make sense, IMO at least. ;)
Comment #4 by deadalnix — 2022-11-10T14:04:56Z
No, that is true in general. For instance, you exemple: char inc(string s, ref int i) { scope(success) i++; if (s == null) return 0; if (s.length > 256) return 1; return s[i]; } Is trivially equivalent to: char inc(string s, ref int i) { char ret; if (s == null) { ret = 0; goto Exit; } if (s.length > 256) { ret = 1: goto Exit; } ret = s[i]; Exit: i++; return ret; }
Comment #5 by kinke — 2022-11-10T15:04:11Z
Almost - this basically corresponds to what the backend generates for the happy path without exception, except that the assignments to `ret` need to be constructions (`TOK.construct`) in order not to break in-place construction of the return value for non-POD return types. Such an invasive rewrite should be feasible in the frontend, but would presumably be quite a bit of work, for little gain IMO.
Comment #6 by deadalnix — 2022-11-27T02:00:33Z
You see what I mean. The backend is not bound by this, it can hold onto the value as an rvalue and return it. An alternative here is to remove empty exception handling code, such as empty finally blocks, or catch blocks that immediately rethrow. In fact, this is probably something we want standalone in LDC's optimizer pipeline, as I'd expect it could find numerous wins.
Comment #7 by robert.schadek — 2024-12-13T19:25:35Z
THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/dmd/issues/20182 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB