Spin2: Get Specific Byte from Word or Long?

JonnyMac · 2020-02-16 23:42

I'm attempting to port some P1 code to the P2. This line throws an error in PNut v34f:

  if (checksum.byte[0] == buffer[BUF_LEN-1])

The reference to .byte[0] causes a problem. I can code around this specific situation, but it would be nice to know that, as in Spin1, I can extract any byte from any word or long.

cgracey · 2020-02-17 00:23

It doesn't work that way. I think I partially forgot how Spin1 worked when I made Spin2.

We have bitfields now, anyway. You could do this:

longvar.[0 addbits 7]

-or-

byte[@longvar]{[index]}

I could add in the old syntax, but there is more to support than before, in order to maintain compatibility with what already exists. This should be supported:

longvar[index].byte[index].[bitfield]

It gets hairy.

JonnyMac · 2020-02-17 00:57

I think I partially forgot how Spin1 worked when I made Spin2.

Yeah, I was up until 3AM this morning bumping up against this (e.g., encod [P2] and >| [P1] work differently). You may consider a P1 review and minimizing differences. The first thing many of us are going to do is port P1 code to the P2.

I could add in the old syntax, ...

I'd like that, because it is very clean and obvious -- which is something I always strive for. The addbits version may be efficient underneath, but it's not obvious to someone reading the code unless they have a very deep knowledge of the P2. Another thing that I just though of -- and that I do quite a bit -- is treat an array of longs (e.g., in a method) as an array of bytes; the .byte[index] syntax makes this very easy.

Roy Eltham · 2020-02-17 02:25

I have to agree with JonnyMac here, the old syntax is clear and simple. This new addbits syntax is not clear at all. I don't even know what "0 addbits 7" means... does it mean 0..7 bits? If so, then why the heck is it "addbits"??? That doesn't lead me to the answer. Only your claim that it somehow equated to reading the low byte of the long got me to think of that.

AJL · 2020-02-17 03:12

Roy Eltham wrote: »

I have to agree with JonnyMac here, the old syntax is clear and simple. This new addbits syntax is not clear at all. I don't even know what "0 addbits 7" means... does it mean 0..7 bits? If so, then why the heck is it "addbits"??? That doesn't lead me to the answer. Only your claim that it somehow equated to reading the low byte of the long got me to think of that.

Roy, I agree.

The 'addbits' seems to relate to how an arbitrary range of pins can be declared for some operations where you declare a base pin for the range.
While this formats makes it easy for the tool to determine what to encode, is there a reason that the range couldn't be represented as a range e.g. 7..0 ?

The fact that this addbits thing is not immediately obvious to some suggests that it isn't the best way to represent it.
I think the old syntax should be supported, even if it makes the job harder for the tool maker.

JonnyMac · 2020-02-17 03:29

This new addbits syntax is not clear at all. I don't even know what "0 addbits 7" means

This operator creates a 10-bit value that specifies the bits to extract. The lower five bits are the base (LSB), and the upper five are the number of bits past the LSB to grab. It seems like the syntax used in the P1 when grabbing inputs would be easy to deal with -- modified like this:

  value := longvar.[MSB..LSB]

I also like that I can flip the LSB and MSB values which saves using ><. This has come in hand on boards with configuration switches that had the natural order swapped.

Rayman · 2020-02-17 03:37

value := longvar[7..0] would be nice...

I actually liked a lot how inb and outb and dirb worked in P1 Spin where you could select bits...

cgracey · 2020-02-17 05:04

Here are the current ways to access variables in Spin2:

	register - type_reg
	-------------------
	regname
	regname.[bitfield]
	regname[index]
	regname[index].[bitfield]

	hub memory - type_loc_???? / type_var_???? / type_dat_???? / type_hub_????
	--------------------------------------------------------------------------
	hubname
	hubname.[bitfield]
	hubname[index]
	hubname[index].[bitfield]

	hub memory - type_size
	----------------------
	BYTE/WORD/LONG[base]
	BYTE/WORD/LONG[base].[bitfield]
	BYTE/WORD/LONG[base][index]
	BYTE/WORD/LONG[base][index].[bitfield]

To access a long variable as a byte variable, you currently need to do one of these:

BYTE[@longvar]
BYTE[@longvar[longindex]]
BYTE[@longvar][byteindex]
BYTE[@longvar[longindex]][byteindex]

I think I could support something like this:

longvar.byte[byteindex]
longvar[longindex].byte[byteindex]

cgracey · 2020-02-17 05:15

Roy Eltham wrote: »

I have to agree with JonnyMac here, the old syntax is clear and simple. This new addbits syntax is not clear at all. I don't even know what "0 addbits 7" means... does it mean 0..7 bits? If so, then why the heck is it "addbits"??? That doesn't lead me to the answer. Only your claim that it somehow equated to reading the low byte of the long got me to think of that.

For pins and bits, a span can be accessed at the two-clock/single-instruction level.

For a bitfield value, bits [4:0] select the base bit and bits [9:5] select how many more bits above the base bit to also select, wrapping around to bit 0 after bit 31. The ADDBITS operator does the masking, shifting, and OR'ing to make the composite bitfield value. This concept must flow to the high-level language for most-efficient code execution.

For a pinfield value, bits [5:0] select the base pin and bits [10:6] select how many additional pins are selected, starting at the base pin and wrapping within the A or B pin group. The ADDPINS operator does the composing of the pinfield value.

Roy Eltham · 2020-02-17 06:32

I still say that "x addpins y" isn't really clear and requires figuring out when you look at the code. The keyword addpins doesn't equate to masking/shifting/or'ing in my brain, like there's no adding going on at all.

makemask(x,y) would at least be more clear what's happening. Maybe just mask(x,y)

cgracey · 2020-02-17 06:40

Roy Eltham wrote: »

I still say that "x addpins y" isn't really clear and requires figuring out when you look at the code. The keyword addpins doesn't equate to masking/shifting/or'ing in my brain, like there's no adding going on at all.

makemask(x,y) would at least be more clear what's happening. Maybe just mask(x,y)

But it's not a mask, actually.

x.[8] 'bit 8 of x
x.[8 addbits 7] 'bits 8..15 of x

It's a concept that is half-way unique, so that prior means of dealing with similar things don't exactly fit. It needs some new treatment.

Tubular · 2020-02-17 07:21

maybe something like PlusNext would help. Or can you only have 7 characters?

cgracey · 2020-02-17 07:27

Tubular wrote: »

maybe something like PlusNext would help. Or can you only have 7 characters?

It can be more than seven chrs. We need one for bits and one for pins.

Roy Eltham · 2020-02-17 07:32

I understand what it does now (after your first explanation), but I mostly just hate the name addbits.
Your own description does say it does masking... which is why I went there.

In Spin1, you could do DIRA[8..15] to me that is more readable than 8 addbits 7. I know it doesn't handle the wrapping, but why must the most common usage be made unclear for that?

Can I still do the Spin1 thing as well? Just for everything instead of only special registers? If I can do myLongVar[8..15] for normal cases and only use the addbits thing for wrap cases, then I'll be fine because I'd never use the wrap case unless forced to.

If we can't do the [x..y] range syntax for at least special registers, then a LOT of Spin1 code will not work in Spin2 without being changed to this unclear ugly syntax.

cgracey · 2020-02-17 07:39

The trouble with [x..y] is that things can be expressed which aren't supportable. ADDBITS keeps things in line with what the hardware does.

Roy Eltham · 2020-02-17 07:48

So those things that are not supported spit out an error.

You had it working fine in Spin1, don't understand how it can't be done exactly the same in Spin2? Just add the new syntax so people can use it for cases the .. syntax doesn't cover.

I get that Spin2 is not compatible with Spin1, but basic stuff like this should be handled the same or it's not correct to even call it Spin. You are making people relearn basic syntax stuff for Spin2, not just new stuff that was added.

cgracey · 2020-02-17 08:05

In Spin1, only special registers were bit-addressable. Now, in Spin2, any variable can be sub-addressed as a bitfield. It changes the syntax, necessarily. I've got it all worked out, with a simple set of rules, but it can't be the same as before. Maybe we just need to accept the difference. It doesn't need to be a huge deal. I just need to get the documentation written, so that the whole picture can be absorbed.

Roy Eltham · 2020-02-17 09:33

I knew that it was special registers only in Spin1, and we wanted it for every long in Spin2. It's great that you have given us the functionality, but it sucks that the syntax changed... especially to this unclear ugly syntax.

I'm surprised others are not joining in on this... because this probably breaks way more existing Spin code than any other change you made.

Maybe it doesn't matter anymore, maybe people don't care anymore about bringing Spin1 stuff over to Spin2 without having to fix a bunch of things that shouldn't need fixing.

I'm not even sure why I am fretting over this at all anymore, I am very unlikely to use Spin2 for P2 coding since it requires that I learn a new language now, and I can just use FlexC with a language I know well already.

Cluso99 · 2020-02-17 10:32

I agree with Roy.
Surely the DIR/IN/OUT syntax of [nn..mm] could be used to use the new instructions. If not, then at least implement it using the old instructions.

There will be enough problems with spin1 incompatibilities. We really need there to be as few as possible. I’ve been hanging out for pnut and spin as I am not wanting it to be a large hubexec program that fastspin gives.

We already have a major exercise in converting P1ASM to P2 with just about every jmp/call/ret requiring explicit fixing/translation. Now Spin2 is so different too.

Do you really want to alienate all the remaining P1 users??? Weve already lost most of them.

Rayman · 2020-02-17 11:35

I’m with Roy on the bit field...
If longvar[8..15] can be made to work, would make things a lot better

Including the reversal of bits shown above.

I often used dirb outb etc. for this on p1 but can’t now that b registers are real...

dMajo · 2020-02-17 14:42

cgracey wrote: »

Roy Eltham wrote: »

I still say that "x addpins y" isn't really clear and requires figuring out when you look at the code. The keyword addpins doesn't equate to masking/shifting/or'ing in my brain, like there's no adding going on at all.

makemask(x,y) would at least be more clear what's happening. Maybe just mask(x,y)

But it's not a mask, actually.

x.[8] 'bit 8 of x
x.[8 addbits 7] 'bits 8..15 of x

It's a concept that is half-way unique, so that prior means of dealing with similar things don't exactly fit. It needs some new treatment.

Can't it, in addition, be also in these forms:

x.[8 to 15]
x.[8..15]

where you find addbits you alredy have the numbers, as opposite when "to" or ".." is found the compiler can do the math: 15-8 (=7)

I am not against the one or the other but for me:
- when the variable have a binary rappresentation eg flags, options, enables ... then addbits is more friendly
- but when you are extracting bits/bytes eg msb lsb from a higher container then x..y or x to y is more readable.

I think both can be supported as it eventually adds only one math operation more in the compiler.

BTW: with x..y or x to y wrapping around must be prevented ie [8..2] or [8 to 2] must be wrong

cgracey · 2020-02-17 15:01

I will make it so you can change the word size of a variable usage, like this:

somevar{.byte|word|long}{[index]}{.[bitfield]}

JonnyMac · 2020-02-17 16:09

To me, this:

  value := x.[15..8]

is far more clear than

  value := x.[8 addbits 7] 'bits 8..15 of x

and is self-commenting. I would be nice to have

  value := x.[8..15]

to save an explicit REV instruction.

Likewise, this syntax

  pinwrite(63..56, x)

is cleaner than

  pinwrite(56 addpins 7, x)

msrobots · 2020-02-17 20:22

I agree also to support both syntaxes, it's a compiler thing ending up at the same opcode.

Mike

cgracey · 2020-02-17 20:27

msrobots wrote: »

I agree also to support both syntaxes, it's a compiler thing ending up at the same opcode.

Mike

If you are talking about constants defining the bitfield, it's easy to support any syntax. Variables make things more complicated.

K2 · 2020-02-17 20:48

@Chip said: For pins and bits, a span can be accessed at the two-clock/single-instruction level.
I'd hate for anything to bust efficiencies like that in the name of backward compatibility.

msrobots · 2020-02-17 21:21

cgracey wrote: »

msrobots wrote: »

I agree also to support both syntaxes, it's a compiler thing ending up at the same opcode.

Mike

If you are talking about constants defining the bitfield, it's easy to support any syntax. Variables make things more complicated.

Fine!

For most uses in P1 we use constants, so allow the syntax with constants and force addpins for variables.

Mike

AJL · 2020-02-17 21:28

K2 wrote: »

@Chip said: For pins and bits, a span can be accessed at the two-clock/single-instruction level.
I'd hate for anything to bust efficiencies like that in the name of backward compatibility.

It's a trade-off between run-time efficiencies and porting efficiencies.

I can see two approaches to the pin range extending beyond the bounds of a long:
1. Throw an error.
2. Render it in two instructions covering the full range, but taking four clocks.

I think most people would be accepting of the first approach as a limitation of the system, given how infrequently such a thing would be attempted intentionally.

whicker · 2020-02-17 22:12

Chip, i support your thinking about addpins.
It's optional, and doesn't break the old syntax, just adds something.

The issue being argued is perhaps misnamed. It's not impeding porting P1 to P2, but making you think a little when going from P2 back to P1.

Roy Eltham · 2020-02-18 01:06

The syntax you use for this in Spin2 should not have any impact on the resulting compiled code, it should not matter if it's a variable or a constant.

You should be able to compile ANYTHING[8..15] to the exact same thing you compile ANYTHING[8 addbits 7] to, right? It means the exact same thing conceptually. It' just a tiny bit of compiler parsing work, I don't get the push back at all.
Even if the user wrote ANYTHING[15..8], that is trivially correctable by swapping the two operands, or you could just throw an error if you don't want to allow it. This is all super trivial compile time stuff that shouldn't matter at all to the end result byte code.

I'm confused why the syntax of this matters at all to the end result byte codes. What am I missing? I don't think it does...

The thing the old syntax does do is make the code significantly more readable and understandable at a glance.

Ariba · 2020-02-18 01:23

Roy Eltham wrote: »

The syntax you use for this in Spin2 should not have any impact on the resulting compiled code, it should not matter if it's a variable or a constant.

You should be able to compile ANYTHING[8..15] to the exact same thing you compile ANYTHING[8 addbits 7] to, right? It means the exact same thing conceptually. It' just a tiny bit of compiler parsing work, I don't get the push back at all.
Even if the user wrote ANYTHING[15..8], that is trivially correctable by swapping the two operands, or you could just throw an error if you don't want to allow it. This is all super trivial compile time stuff that shouldn't matter at all to the end result byte code.

I'm confused why the syntax of this matters at all to the end result byte codes. What am I missing? I don't think it does...

The thing the old syntax does do is make the code significantly more readable and understandable at a glance.

Your bit range is constant. The problem is: ANYTHING[mypin addbits 7] where mypin is a variable, maybe passed as a parameter to a methode.

Edit:
Maybe ANYTHING[mypin+7 .. mypin] will work, the rule would be: It must be the same variable and a constant offset.

Spin2: Get Specific Byte from Word or Long?

Comments