Opened 11 years ago
Closed 10 years ago
#1094 closed defect (fixed)
define-inline does not respect inline-limit
Reported by: | johnwcowan | Owned by: | evhan |
---|---|---|---|
Priority: | major | Milestone: | 4.10.0 |
Component: | compiler | Version: | 4.8.x |
Keywords: | Cc: | ||
Estimated difficulty: |
Description ¶
If you define a recursive procedure with define-inline
, csc 4.8.0.4 loops.
(define-inline (foo x) (if (= x 0) 0 (+ (foo (- x 1)) 1))) (foo 32)
Compile with csc -t
, wait forever. This is not the case with
(declare (inline foo)) (define (foo x) (if (= x 0) 0 (+ (foo (- x 1)) 1))) (foo 32)
which suppresses the inlining.
Change History (3)
comment:1 Changed 11 years ago by
Milestone: | someday → 4.10.0 |
---|
comment:2 Changed 10 years ago by
Owner: | set to evhan |
---|---|
Status: | new → assigned |
The define-inline
feature is pretty much totally distinct from procedure inlining (to which (declare (inline ...))
and inline-limit
apply) -- the former is strictly mechanical and takes place during canonicalization, whereas the latter happens later on when the optimizer decides whether or not to inline a predefined procedure or not. Also, inline-limit
is a threshold of procedure size, which can't be easily applied to define-inline
forms.
To prevent loops, we could add a depth limit for nested define-inline
expansions. Or, this may just be a documentation bug that we fix by making clear that define-inline
is a naive transformation and if you're not careful, recursive inlines may loop forever. I'd prefer the latter.
Thoughts?
comment:3 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
I've added a note to define-inline
's documentation that inline-limit
doesn't apply to it in 5d102c31cef490e6e3d3bd8f0044801ed4e50d0e.
A cutoff for nested define-inline
expansion should really be a separate feature request, as described above. For now, I'm going to call this a documentation bug and mark it resolved. Please open a new enhancement request if such a feature would be valuable to you.
A bit silly, but if we can easily fix it, that should probably go into 4.10.0