loop should retain primitive int or float without widening

Description

In the following example:

the loop-local starts as an int when just a local but is widened to a long in the recur. It should be possible to retain the primitive int (or alternately float) type on the recur, rather than widening to long (or double).

The compiler code that is promoting the int seems to be:
https://github.com/clojure/clojure/blob/a19c36927598677c32099dabd0fdb9d3097df259/src/jvm/clojure/lang/Compiler.java#L6377-L6380

Proposed: remove useless widening on loop bindings

Patch: clj-1905-2.patch

Screened by: Alex Miller. My main open question here is: do we want to support primitive int/float loop vars?

Environment

Possibly older Clojure versions (but not verified).

Activity

Show:
Nicola Mometto
March 29, 2016, 6:19 PM

I don't think we need to worry about fn recur targets at all, given that the only possible primitive bindings there are either long or double, and int/floats would get widened anyway, but good point, the tests in a patch for this ticket need to be sure that case is indeed handled.

RE: floats – I recall people complaining about bad support for floats when using clojure for graphical processing.

Even if admittedly a weak argument, I'm always of the idea that we should strike to be as consistent as possible. I don't think anybody would expect let/loop locals to behave differently, or differences between primitive types (other than the documented limitation about long/double being the only working prim types for fn arguments/return vals)

Alex Miller
March 29, 2016, 6:30 PM

I'll leave this one in here but I'm going to treat it as an enhancement to current behavior. I think there's a good chance that Rich will just say this is working as intended.

I don't think the example is a very good one though and would welcome a better example. The reservations regarding unchecked-inc-int do not seem correct or valid to me (as usage should be fine on longs and is not designed for perf reasons anyways). A good example would should usage of a Java api in a loop where int-indexing and int-math gives better performance.

Nicola Mometto
March 30, 2016, 2:51 PM

I edited the title as the bug is in `loop`, not `recur`

Nicola Mometto
April 2, 2016, 3:55 PM

Attached a patch that removes the useless widenings done by the compiler on loop bindings, here's a benchmark demonstrating the speedup gained when areducing over int-arrays:

Before patch:

After patch:

Alex Miller
December 12, 2018, 6:46 PM

-2 patch rebased to apply to master, no changes, retains attribution

Assignee

Unassigned

Reporter

Renzo Borgatti

Approval

Screened

Patch

Code and Test

Fix versions

Affects versions

Priority

Minor