Summary
Flonum unboxing too agressive
Metadata
- Id: e3295c60b376ded395797514d17cb84525fe08d9
- Trac id: 1624
- Type: defect
- Reporter: sjamaan
- Owner:
- Cc:
- Status: closed
- Component: compiler
- Estimated difficulty: medium
- Resolution: fixed
- Priority: critical
- Milestone: 5.2
- Version: 5.1.0
- Changetime: 2019-06-29 14:29:21 UTC
- Created: 2019-06-23 14:47:57 UTC
- Keywords: lfa2
Description
Found by Sven Hartrumpf. The following program will segfault:
(define (test-it)
(let loop ((done #f))
(if done
'()
(cons (or (read) 0.0) (loop #t)))))
(print (test-it))
When run with a DEBUGBUILD CHICKEN, you'll see it fails with a nice panic due to a low-level assertion error triggered by a C_flonum_magnitude call when you enter a fixnum:
[panic] Low-level type assertion C_blockp((C_word)C_VAL1(C__PREV_TMPST.n1))=#t failed at test.c:176 - execution terminated
Generated code failing:
/* k150 in loop in test-it in k131 in k128 in k125 */
static void C_ccall f_152(C_word c,C_word *av){
C_word tmp;
C_word t0=av[0];
C_word t1=av[1];
C_word t2;
C_word t3;
C_word t4;
double f0;
C_word *a;
C_check_for_interrupt;
if(C_unlikely(!C_demand(C_calculate_demand(8,c,2)))){
C_save_and_reclaim((void *)f_152,2,av);}
a=C_alloc(8);
f0=C_flonum_magnitude((C_truep(t1)?t1:lf[1])); /// <--- offending code
t2=C_flonum(&a,f0);
t3=(*a=C_CLOSURE_TYPE|3,a[1]=(C_word)f_159,a[2]=((C_word*)t0)[2],a[3]=t2,tmp=(C_word)a,a+=4,tmp);
C_trace(C_text("test.scm:5: loop"));
t4=((C_word*)((C_word*)t0)[3])[1];
f_141(t4,t3,C_SCHEME_TRUE);}
Most likely introduced by 79cf7427638eebf695f2be54731bb6bbf4d0fff2.
ALSO: In the generated code in this case, it unboxes and immediately boxes the flonum. There's no operation in between, so that's wasteful.
It only makes sense with >1 operations (though with exactly 1 it should be okay too but adds no perf improvement)
Changes and comments
[2019-06-23 15:50:26 UTC] felix changed difficulty from hard to medium
[2019-06-23 15:50:26 UTC] felix wrote:
The offending code seems to be lfa2.scm:349, the rule for walking `if`/`##core#cond` nodes. The result type is always taken from the 2nd branch, but should instead be the union of the result types of both branches.
[2019-06-24 11:21:48 UTC] sjamaan changed description
[2019-06-25 22:49:58 UTC] felix wrote:
AFAICT, the transformation in this pass doesn't create redundant boxing. I think, in this particular example the pointless boxing/unboxing is an artifact of the completely wrong type computed by walking the conditional.
[2019-06-29 14:29:21 UTC] sjamaan changed status from new to closed
[2019-06-29 14:29:21 UTC] sjamaan set resolution to fixed
[2019-06-29 14:29:21 UTC] sjamaan wrote:
Fixed by 9413c4bf1b4b0c95e60ba12e082f4e03b4c0f8ef