View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0025440 | FPC | Compiler | public | 2013-12-18 19:29 | 2013-12-21 11:47 |
Reporter | Mark Morgan Lloyd | Assigned To | Sergei Gorelkin | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Platform | SPARC | OS | Linux | ||
Product Version | 2.7.1 | ||||
Fixed in Version | 3.0.0 | ||||
Summary | 0025440: On SPARC, spurious range error when decrementing a smallint class field. | ||||
Description | When the a smallint field with the value -1 is decremented, a range error is raised. Does not appear to affect ordinary variables. Affects SPARC Linux, does not affect x86 or PPC Linux, other platforms untested. | ||||
Steps To Reproduce | Run attached program. Correct output should be numbers 1 0 -1 followed by OK, on SPARC the OK is replaced by an error 201. | ||||
Tags | No tags attached. | ||||
Fixed in Revision | 26255,26258 | ||||
FPCOldBugId | |||||
FPCTarget | |||||
Attached Files |
|
|
test_range.pas (596 bytes)
program TestRange; {$mode objfpc } {$Q- } {$R+ } type TSynPasSynRange = class // (TSynCustomHighlighterRange) protected FPasFoldFixLevel: Smallint; public procedure DecLastLinePasFoldFix; end; procedure TSynPasSynRange.DecLastLinePasFoldFix; begin dec(FPasFoldFixLevel); end; var z: TSynPasSynRange; begin z := TSynPasSynRange.Create; z.FPasFoldFixLevel := 1; WriteLn(z.FPasFoldFixLevel); z.DecLastLinePasFoldFix; WriteLn(z.FPasFoldFixLevel); z.DecLastLinePasFoldFix; WriteLn(z.FPasFoldFixLevel); z.DecLastLinePasFoldFix; WriteLn('OK'); z.Free end. |
|
Caused by two reasons: - inc(x)/dec(x) with range checking enabled are "optimized" with taking address of x. They were using untyped address, causing loss of alignment and generating large and slow code with unaligned access. This is fixed in r26256 and improves all alignment-sensitive targets. - tcg.a_load_ref_reg_unaligned was always zero-extending the result, while it is expected to sign-extend if loading a signed value. This is fixed in r26255. |
|
Is this safe if you call inc() on an unaligned field in a packed record? I don't see where the alignment is preserved in the new code. In fact, it doesn't seem like there is a possibility to create an "unaligned" pointer in the compiler currently other than by using e.g. a voidpointer (tcgderefnode.pass_generate_code always forces natural alignment) |
|
I've tested Sergei's fix on SPARC against the example code that I supplied and it appears OK. However I lack the expertise to decide that the issue should be closed in light of Jonas's questions. |
|
Jonas is right, as always :-) It is impossible to preserve, or even determine the alignment during pass 1, so using a voidpointer is only safe way. However it is possible to increase the node complexity at which the optimization happens, so inc/dec on e.g. record fields do not use the optimization. This is done in r25258. |
|
I'm definitely not always right. Case in point, there is a way to make pointer-based accesses unaligned in pass 1: create the equivalent of unaligned(ptr^). Determining the alignment in pass 1 for all cases is however indeed another matter :/ We could add an optimization for certain simple cases (local variables, parameters), but it's not very clean. |
|
Tested and believed OK. Many thanks for the fix. |
Date Modified | Username | Field | Change |
---|---|---|---|
2013-12-18 19:29 | Mark Morgan Lloyd | New Issue | |
2013-12-18 19:29 | Mark Morgan Lloyd | File Added: test_range.pas | |
2013-12-20 18:25 | Sergei Gorelkin | Note Added: 0071985 | |
2013-12-20 18:26 | Sergei Gorelkin | Fixed in Revision | => 26255,26256 |
2013-12-20 18:26 | Sergei Gorelkin | Status | new => resolved |
2013-12-20 18:26 | Sergei Gorelkin | Fixed in Version | => 2.7.1 |
2013-12-20 18:26 | Sergei Gorelkin | Resolution | open => fixed |
2013-12-20 18:26 | Sergei Gorelkin | Assigned To | => Sergei Gorelkin |
2013-12-20 18:36 | Jonas Maebe | Note Added: 0071986 | |
2013-12-20 23:19 | Mark Morgan Lloyd | Note Added: 0071991 | |
2013-12-20 23:19 | Mark Morgan Lloyd | Status | resolved => feedback |
2013-12-20 23:19 | Mark Morgan Lloyd | Resolution | fixed => reopened |
2013-12-21 11:42 | Sergei Gorelkin | Note Added: 0071993 | |
2013-12-21 11:42 | Sergei Gorelkin | Fixed in Revision | 26255,26256 => 26255,26258 |
2013-12-21 11:42 | Sergei Gorelkin | Status | feedback => resolved |
2013-12-21 11:42 | Sergei Gorelkin | Resolution | reopened => fixed |
2013-12-21 11:46 | Jonas Maebe | Note Added: 0071994 | |
2013-12-21 11:47 | Mark Morgan Lloyd | Note Added: 0071995 | |
2013-12-21 11:47 | Mark Morgan Lloyd | Status | resolved => closed |