add hal_extend_int function

[rmu75]

helper function to deal with wrap around and extension of lower-width counters to 64bit ints.

[BsAtHome]

The explanation does not tell us what the function is supposed to do, why it exists and how it accomplished the desired outcome. Neither does it describe arguments, like what values old, newlow and nbits represent.

Also, IMO, the name hal_extend_int() is a misnomer for what is actually performed.

[rmu75]

The explanation does not tell us what the function is supposed to do, why it exists and how it accomplished the desired outcome. Neither does it describe arguments, like what values old, newlow and nbits represent.

Also, IMO, the name hal_extend_int() is a misnomer for what is actually performed.

/** hal_extend_int() extends a counter value with nbits to 64 bits.
    Increments between updates need to be less than 2**(nbits-1).
    Call with current 64bit counter value to be updated, new
    lower-bit counter value and number of bits of counter.
    @returns new counter value.
    Code by Jeff Epler. */

hmm. I guess my description presupposes some context. It could certainly be more verbose. Regarding the name, I didn't and still don't have a good idea how to name it. Feel free to suggest something. @andypugh opinion re. name?

[BsAtHome]

hmm. I guess my description presupposes some context. It could certainly be more verbose.

Yes, and that is exactly the problem you yourself pointed out and warned about when adding "smart code".

Regarding the name, I didn't and still don't have a good idea how to name it. Feel free to suggest something. @andypugh opinion re. name?

What is its primary function? Usually, name follows function.

[BsAtHome]

Just one more thing... This whole routine is being added because the usual algorithm has one or two conditionals in performing a fixup.

Then why is this being added as a subroutine involving two non-local jumps (call and ret), disregarding possible PLT indirection, and at least two stack operations, assuming the arguments are register passed? I fail to see the advantage in that.

[rmu75]

I think the "cleverness" is adequately addressed in the code comment.

I propose this code to be added to have one (and only one) correct implementation that can deal with all cases, not to avoid branches or whatever. Correcctness trumps speed.

Unconditional jumps are cheap. I don't think a macro is warranted here, neither pollution of the header file with implementation of an inline function or a ".inc" code include file.

[BsAtHome]

I think the "cleverness" is adequately addressed in the code comment.

We apparently have very different ideas of "adequately addressed".

I propose this code to be added to have one (and only one) correct implementation that can deal with all cases, not to avoid branches or whatever. Correcctness trumps speed.

I can accept correctness, in code. But I fail to see that the implementation is actually correct in the greater context.

Unconditional jumps are cheap.

No, they are not cheap. These are not simple jumps and involve stack frames. If this code is PLT indirected, then there is one more indirect jump involved too.

I don't think a macro is warranted here, neither pollution of the header file with implementation of an inline function or a ".inc" code include file.

It shouldn't be a macro as such. It should be an inline function that is always inlined. Then the compiler has a chance of performing optimizations and is not forced to create and tear down a stack frame construct (requiring flushing local register scheduling).

[BsAtHome]

The prototype is effectively superfluous unless you try to link in a system that does both not inline and not define that static function locally as static. But, since we are using a C compiler that actually does inline, I do not see the point of the prototype.

Or is there something that I missed?

[BsAtHome]

You may think that I am pedantic here, but I'm just reading from a blanked mind (like one from 5 years in the future) and try to fit that into why I would need this function.

It still does not explain what the arguments old and newlow actually are in the context of extending a counter. Which of these arguments is the counter?
The names suggest that there is something "old", but it is unclear what is old. There also is something "new" and apparently also "low" at the same time. But I cannot fathom, from the description, how these names fit into the algorithm.

BTW, the name hal_extend_counter() does seem much more appropriate, but there is more going on than just extending the counter. It is calculating a counter increment from the difference of the arguments and adding that difference to one of the arguments. This exploits the behaviour of unsigned verses two's complement calculations. It uses smaller values as source into a larger register set as if the calculation was performed on a "nbits" register in an unsigned operation (exploiting overflow behaviour) casted to a two's complement result. The final calculation is furthermore (ab)using the unsigned/signed duality. How exactly is this an "easy" algorithm to follow?

[rmu75]

See comment next to prototype. This comment here in the impl explains what the code is doing and is inside. IDE will display the other comment if oyu hover over a function call.

In fact it doesn't matter if you work with signed or unsigned counter values, the important thing is that the difference has the proper sign and is calculated with width nbits. instead of figuring out the high bits, shifting it up gets proper overflow behaviour and the "extra (0) bits" towards LSB don't matter.