Skip to content

Commit

Permalink
remove special-case lowering for for i = a:b loops (ref #5355)
Browse files Browse the repository at this point in the history
this fixes some edge-case loops that the special lowering did not
handle correctly.

colon() now checks for overflow in computing the length, which avoids
some buggy Range1s that used to be possible.

this required some changes to make sure Range1 is fast enough:
specialized start, done, next, and a hack to avoid one of the checks and
allow better inlining.

in general performance is about the same, but a few cases are actually
faster, since Range1 is now faster (comprehensions used Range1 instead
of the special-case lowering, for example). also, more loops should be
vectorizable when the appropriate LLVM passes are enabled. all that
plus better correctness and a simpler front-end, and I'm sold.
  • Loading branch information
JeffBezanson committed Jan 17, 2014
1 parent efde013 commit 0860767
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 43 deletions.
31 changes: 28 additions & 3 deletions base/range.jl
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,38 @@ immutable Range1{T<:Real} <: Ranges{T}
len::Int

function Range1(start::T, len::Int)
if !(len >= 0); error("length must be non-negative"); end
if len < 0; error("length must be non-negative"); end
new(start, len)
end
Range1(start::T, len::Integer) = Range1(start, int(len))

# TODO: this is a hack to elide the len<0 check for colon.
# should store start and stop for integer ranges instead
Range1(start::T, len::Integer, _) = new(start, int(len))
end
Range1{T}(start::T, len::Integer) = Range1{T}(start, len)

function colon{T<:Integer}(start::T, step::T, stop::T)
step != 0 || error("step cannot be zero in colon syntax")
Range(start, step, max(0, 1 + fld(stop-start, step)))
Range{T}(start, step, max(0, 1 + fld(stop-start, step)))
end

colon{T<:Integer}(start::T, stop::T) =
Range1(start, max(0, stop-start+1))
Range1{T}(start, ifelse(stop<start, 0, int(stop-start+1)))

if Int === Int32
colon{T<:Union(Int8,Int16,Int32,Uint8,Uint16)}(start::T, stop::T) =
Range1{T}(start,
ifelse(stop<start, 0,
checked_add(checked_sub(convert(Int,stop),convert(Int,start)),1)),
0) # hack to elide negative length check
else
colon{T<:Union(Int8,Int16,Int32,Int64,Uint8,Uint16,Uint32)}(start::T, stop::T) =
Range1{T}(start,
ifelse(stop<start, 0,
checked_add(checked_sub(convert(Int,stop),convert(Int,start)),1)),
0) # hack to elide negative length check
end

function colon{T<:Real}(start::T, step::T, stop::T)
step != 0 || error("step cannot be zero in colon syntax")
Expand Down Expand Up @@ -151,6 +170,12 @@ next{T}(r::Range{T}, i) = (oftype(T, r.start + i*step(r)), i+1)
next{T}(r::Range1{T}, i) = (oftype(T, r.start + i), i+1)
done(r::Ranges, i) = (length(r) <= i)

# though these look very similar to the above, for some reason LLVM generates
# much better code for these.
start{T<:Integer}(r::Range1{T}) = r.start
next{T<:Integer}(r::Range1{T}, i) = (i, oftype(T, i+1))
done{T<:Integer}(r::Range1{T}, i) = i==(r.start+r.len)

==(r::Ranges, s::Ranges) = (r.start==s.start) & (step(r)==step(s)) & (r.len==s.len)
==(r::Range1, s::Range1) = (r.start==s.start) & (r.len==s.len)

Expand Down
56 changes: 16 additions & 40 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -1722,46 +1722,22 @@
(let ((X (caddr (cadr e)))
(lhs (cadr (cadr e)))
(body (caddr e)))
(if
(and (pair? X) (eq? (car X) ':) (length= X 3))
;; (for (= lhs (: a b)) body)
(let* ((cnt (gensy))
(a (cadr X))
(b (caddr X))
(lim (if (number? b) b (gensy))))
`(scope-block
(block
(= ,cnt ,(expand-forms a))
,.(if (eq? lim b) '() `((= ,lim ,(expand-forms b))))
(break-block loop-exit
(_while (call (top <=) ,cnt ,lim)
(scope-block
(block
;; NOTE: enable this to force loop-local var
#;(local ,lhs)
(= ,lhs ,cnt)
(break-block loop-cont
,(expand-forms body))
(= ,cnt (call (top convert)
(call (top typeof) ,cnt)
(call (top +) 1 ,cnt))))))))))

;; (for (= lhs X) body)
(let ((coll (gensy))
(state (gensy)))
`(scope-block
(block (= ,coll ,(expand-forms X))
(= ,state (call (top start) ,coll))
,(expand-forms
`(while
(call (top !) (call (top done) ,coll ,state))
(scope-block
(block
;; NOTE: enable this to force loop-local var
#;,@(map (lambda (v) `(local ,v)) (lhs-vars lhs))
,(lower-tuple-assignment (list lhs state)
`(call (top next) ,coll ,state))
,body))))))))))
;; (for (= lhs X) body)
(let ((coll (gensy))
(state (gensy)))
`(scope-block
(block (= ,coll ,(expand-forms X))
(= ,state (call (top start) ,coll))
,(expand-forms
`(while
(call (top !) (call (top done) ,coll ,state))
(scope-block
(block
;; NOTE: enable this to force loop-local var
#;,@(map (lambda (v) `(local ,v)) (lhs-vars lhs))
,(lower-tuple-assignment (list lhs state)
`(call (top next) ,coll ,state))
,body)))))))))

'+= lower-update-op
'-= lower-update-op
Expand Down
47 changes: 47 additions & 0 deletions test/ranges.jl
Original file line number Diff line number Diff line change
Expand Up @@ -103,3 +103,50 @@ r = (-4*int64(maxintfloat(is(Int,Int32) ? Float32 : Float64))):5

# avoiding intermediate overflow (#5065)
@test length(1:4:typemax(Int)) == div(typemax(Int),4) + 1

# overflow in length
@test_throws 0:typemax(Int)
@test_throws typemin(Int):typemax(Int)
@test_throws -1:typemax(Int)-1

# parity between ranges and for loops (see issue #5355)

@test length(2.0^53:(2.0^53+2)) == 3
let s = 0
r = 2.0^53:(2.0^53+2)
for i in r
s += 1
@test s <= 3
end
@test s == 3

s = 0
for i in 2.0^53:(2.0^53+2)
s += 1
@test s <= 3
end
@test s == 3
end

let s = 0
# loops ending at typemax(Int)
for i = (typemax(Int)-1):typemax(Int)
s += 1
@test s <= 2
end
@test s == 2

s = 0
for i = (typemax(Int)-2):(typemax(Int)-1)
s += 1
@test s <= 2
end
@test s == 2

s = 0
for i = typemin(Int):(typemin(Int)+1)
s += 1
@test s <= 2
end
@test s == 2
end

0 comments on commit 0860767

Please sign in to comment.