[Chicken-hackers] [PATCH] A few small performance and scrutiny warning improvements to assignments

[Peter Bex]

Hello all,

The other day, it occurred to me that when you assign to a variable
or a slot in a vector, it's possible to avoid a mutation tracking
check, if at compile time the value is known to be immediate.
Instead, we can emit code to write the value directly to the data
pointer + index.  This will save one "if (!immediatep(val))" check
at runtime (in C_mutate2) and result in the best code possible for
mutating a slot.

I quickly checked the code and figured out that the compiler already
does this when you set! a variable to some value, but only in the
limited case where that value is a quoted immediate value.

The scrutinizer knows more about values, so I decided to add this
information.  I had some trouble finding out how to communicate
information from the scrutinizer to the compiler itself.  This needs
to be done on the specific set!-expression, because the variable or
its type doesn't affect this, but the value you set! does.

Because the choice on how to set! something happens pretty late in
the compilation process, this can't happen inside the scrutinizer
directly.  So I decided to simply add a second optional argument
to the node tree.  If it's present and #t, the value is known to
be immediate and we can use ##core#setglobal instead of
##core#setglobal, and ##core#update_i instead of ##core#update.
In terms of C, this means we'll use C_set_block_item(x, i, y) or lf[i]=y
instead of C_mutate2(x+i, y) or C_mutate2(lf[i], x).
This is what the patch 0001 does.

The next one, patch 0002, is similar: it rewrites vector-ref and
vector-set! so they use ##sys#slot and ##sys#setslot if the index is
known to exist in the vector, and ##sys#setislot if the value is also
known to be immediate.  It's pretty simple, it uses what's already
there, but that makes it somewhat surprising, because the special
casing was only really intended to return a new type.  Now it also
modifies the node tree.  This needs to be refactored, I think, but
I didn't want to do it in this set of patches because I was moving
plenty of stuff around already.

Patch 0003 is a simple modification based on the preceding one: it
gives a scrutiny warning when you try to set or ref a vector at an
index that is known not to exist.  For completeness, I also did this
for list-ref, list-tail, take and drop.  All that's really needed is to
pass the "loc" argument to the registered special-case procedures
and emit the warning when this situation is detected (we already
did so).  Unfortunately the code structure make this needlessly hard.
I had to copy the definition of "report" and take out the location-name
and node-source-prefix procedures.  We should also really take a
good look at how we can refactor this because I don't like how this
currently works.

Finally, patch 0004 is a natural progression of patch 0003; when I wrote
the tests for patch 0003, I noticed that the scrutinizer "lost" the
sizes of the vectors.  This is due to the smash-component-types code.
This erases the specific types of each vector's or lists's contents.
This is too pessimistic: A vector cannot change in size.  So I modified
smash-component-types to keep the size but only change each component's
type to '*.  I think this last change is the most worthwhile of the
bunch as it can help find mistakes, and it will (AFAICT) give off no
false positive.

I worked on this in master, because I wanted to run Mario's benchmark
suite on it, which doesn't currently work on CHICKEN 5.  Unfortunately,
these changes don't have any measurable effect on the benchmark suite :(

The other advantage of applying these to master is that we can more
thoroughly test the consequences of the changes.  If there's anything I
overlooked, these changes *could* cause subtle problems, and Salmonella
might (hopefully) catch that.  So I really think that it is worthwhile
to add these to master as well as chicken-5.

Cheers,
Peter

0001-Do-not-track-set-to-known-to-be-immediate-values.chicken-5.patch
Description: Text Data

0002-Special-case-vector-ref-set-to-sys-set-i-slot-when-i.chicken-5.patch
Description: Text Data

0003-Add-scrutiny-warning-for-bad-indexing-into-vectors-a.chicken-5.patch
Description: Text Data

0004-Keep-vector-length-when-smashing-component-types.chicken-5.patch
Description: Text Data

0001-Do-not-track-set-to-known-to-be-immediate-values.master.patch
Description: Text Data

0002-Special-case-vector-ref-set-to-sys-set-i-slot-when-i.master.patch
Description: Text Data

0003-Add-scrutiny-warning-for-bad-indexing-into-vectors-a.master.patch
Description: Text Data

0004-Keep-vector-length-when-smashing-component-types.master.patch
Description: Text Data

signature.asc
Description: Digital signature