# do_consttovar creates a persistent register temp that is never deleted, so it can be reused mid-loop ## Summary `do_consttovar` (`compiler/optcse.pas`) hoists a frequently used constant or symbol address into a `tt_persistent` temp assigned at procedure entry, and replaces every use with a tempref. It creates a `deleteblock` for the matching `ctempdeletenode` — and never puts one in it. For register temps that delete node is what emits the final `a_reg_sync`. Without it the register allocator releases the register after the last _textual_ use. If that use sits inside a loop, the register gets handed to another value before the back edge, and the next iteration reads the wrong thing. The protection normally comes from `tcgwhilerepeatnode.sync_regvars` (`compiler/ncgflw.pas:133`), but that is gated on `cs_opt_regvar and not pi_has_label`, so a routine that merely inlines a function containing a `label`/`goto` loses it. `-OoNOREGVAR` loses it too. Patch below: two lines, populating the block that is already there. ## Reproducer `consts5.pas`, attached, no external dependencies: - `fpc -O2 -Twin64 -Paarch64 consts5.pas` → EAccessViolation - `fpc -O2 -OoNOCONSTS ...` → `assertions=1100002 fails=0 / OK` - `fpc -O1 ...` → OK The ingredients are a global whose address is used repeatedly in a loop, an inlined function containing `label`/`goto` (setting `pi_has_label` in the caller), and enough boolean temporaries after the last use of the global. `consts2.pas` covers the `-OoNOREGVAR` path without any label. ## What happens Measured on the routine where this first showed up (mORMot2's `TTestCoreBase.Utf8Slow`, 22791 instructions, aarch64-win64): - the address of the global `CurrentAnsiConvert` is loaded once, at the top, into x20 - x20 is written 151 times in the routine; 2 of those load the address, the rest are boolean temporaries (`cset w20,eq`, `uxtb w20,w0`) - from the `-ar` annotated build: x20 allocated at 143905, last use 177893, **released at 177895 — inside the loop**, reallocated to a boolean at 178009, back edge at 179876 - second iteration reads `ldr x0,[x20]` with x20 = 1 and faults A vectored exception handler confirms the fault: `STATUS_ACCESS_VIOLATION`, read, address 1 — a `w20` write zero-extends, so a boolean `true` lands as address 1. With `-OoNOCONSTS` the same address is loaded 12 times into x0 (volatile) instead, and everything works. Note that `try..except` is not part of the mechanism, although it changes register pressure enough to make the failure appear or disappear (`is_regvar` returns false for all locals when `pi_uses_exceptions` is set, `symsym.pas:1988`). The attached reproducer has no `try..except`. ## Patch ```diff --- a/compiler/optcse.pas +++ b/compiler/optcse.pas @@ -804,6 +804,12 @@ constentries[i].valuenode.resultdef.size,tt_persistent,true); addstatement(creates,constentries[i].temp); addstatement(creates,cassignmentnode.create_internal(ctemprefnode.create(constentries[i].temp),constentries[i].valuenode)); + { release the temp at the end of the routine: for register + temps this emits a final a_reg_sync so the register + allocator keeps the register live across loop back edges + even when regvar loop syncing is unavailable + (-OoNOREGVAR or pi_has_label) } + addstatement(deletes,ctempdeletenode.create(constentries[i].temp)); current_filepos:=old_current_filepos; foreachnodestatic(pm_postprocess,rootnode,@replaceconsts,@constentries[i]); inc(fpu_regs_assigned); @@ -829,6 +835,7 @@ addstatement(creates,constentries[i].temp); addstatement(creates,cassignmentnode.create_internal(ctemprefnode.create(constentries[i].temp), caddrnode.create_internal(constentries[i].valuenode))); + addstatement(deletes,ctempdeletenode.create(constentries[i].temp)); current_filepos:=old_current_filepos; foreachnodestatic(pm_postprocess,rootnode,@replaceconsts,@constentries[i]); inc(int_regs_assigned); ``` `tcgtempdeletenode.pass_generate_code` (`ncgbas.pas:660`) emits the sync for `LOC_CREGISTER` only `if not(cs_opt_regvar...) or (pi_has_label...)`, with the comment "make sure the register allocator doesn't reuse the register e.g. in the middle of a loop". So in the common case the patch costs nothing, and in exactly the cases that lose `sync_regvars` it restores the protection the authors intended. ## Verification On 416b51be87 plus this patch, aarch64-win64: - `consts5.pas` and `consts2.pas` at `-O2`: pass - mORMot2 regression suite, plain `-O2`, no workaround flags: `TTestCoreBase` completes with 118,615,145 assertions and the same 26 failures as an `-OoNOCONSTS` baseline build. Before the patch it died with an access violation. The failure profile of the full suite matches the baseline run (26 core base, 640 core process, 3 compression, 4 sqlite). - bootstrap is stable: compiling the patched compiler with itself twice gives bit-identical binaries (sha256 9AD57ADCE14A38CAE9AE41EA946F5809599EC6610C35F85BE4AFF2564E85CCCB). ## Scope Nothing here is aarch64-specific — the missing delete affects the float-constant path the same way, and any target reaching this code with `-OoNOREGVAR` or `pi_has_label`. aarch64-win64 is simply where it was hit. ## Not done The FPC test suite has not been run against this patch; only the reproducers, the mORMot2 suite and a triple bootstrap. Someone should run it before this is merged. [fpc-consttovar-repro.zip](/uploads/7c811a693c2935d13064a40c4bebd9ad/fpc-consttovar-repro.zip)