ASM finer points

Go To Last Post
22 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello All:

I'm trying to come up to speed on AVR assembler functions. Below is a snippet from an AVR tutorial.
-----
ldi r16,1<<ADSC
out ADCSRA,r16

In this example, ADSC is a certain bit in Port ADCSRA, both defined somewhere in the header file for this AVR. In that case it starts an AD conversion (in most cases bit 6), if written to the Port ADCSRA. The expression with << causes that a One (binary 0000.0001) is shifted left six times and gets to 0100.0000. This number is written to the port ADCSRA and causes an AD conversion start.

Using this notation has the advantage, that the exact location of the ADSC bit in the ADCSRA port doesn't have to be known exactly. Its location can even change from one header file to another, without changing the source code. Compared to the formulation

ldi r16,0b01000000
out 0x06,r16

the transparency is much higher. And not having to count binary zeroes is another advantage.

------- end snippet -----

Unless there is a convention I don't know about, I disagree with the above. ADSC is actually LESS transparent in the first example, because its value is defined elsewhere.

The tutorial implies that ADSC has a value of 6, but nowhere in the example is this explicitly shown.

If one maintains ADSC as an abstraction, performing the shift as an assembler directive during the ldi muddies the code, AND the bit position of ADSC is still not known. IMHO, ADSC should be defined (elsewhere) as 0b01000000 or 1<<0x06 rather than what "appears" to be 0x06. To increase transparency, the label could be changed to ADSCbit6, to differentiate it from situations where ADSC is NOT bit 6.

Obviously, defining a constant once eliminates the need, and danger, of changing multiple absolute values as hardware changes, but most of what is said in the tutorial seems to be a weak excuse to show the shift left function.

Sorry if this is too esoteric, (or just plain picky):-) but I want to make sure I have a solid foundation, rather than get misled by a poorly written bit of instruction.

Cheers,

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

well, seeing you write (1<<ADSC) to ADCSRA, i immediately know what you intended to do there, without having to open the datasheet.

You should not be counting bits anyway, as the name in the datasheet (ADSC) corresponds to the definitions in the include file. Why mess with numbers when there are symbols.

Why would i care that ADSC is bit 6 ?

In my opinion, and i think most users here will agree, this notation does not clutter the readability at all, on the contrary, it's much easier to comprehend, without having to cross reference with the datasheet all the time.

For example :

UCSR1B = (1<<RXEN1)|(1<<TXEN1)|(1<<RXCIE1);
UCSR1C = (1<<UCSZ01)|1<<UCSZ00);

OR 

UCSR1B = 0b10011000;
UCSR1C = 0b00000110;

"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it"

Last Edited: Fri. Mar 25, 2011 - 07:18 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

thygate wrote:
well, seeing you write (1<<ADSC) to ADCSRA, i immediately know what you intended to do there, without having to open the datasheet.

You should not be counting bits anyway, as the name in the datasheet (ADSC) corresponds to the definitions in the include file. Why mess with numbers when there are symbols.

Why would i care that ADSC is bit 6 ?


Exactly what I would have replied. The symbolic names convey the important information. The associated numbers are usually unimportant details. When I see code that calls a subroutine do I gain more understanding by seeing the name of the subroutine or its address?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

To increase transparency, the label could be changed to ADSCbit6, to differentiate it from situations where ADSC is NOT bit 6.

It could. And, >>you<< could do that too.
As it is, over the years the include file <=> datasheet consistency on these names has been getting better and better. So if you see ADSC in your code, you can find it in the datasheet and vice-versa.

Now, no matter how you do the naming you could still be wrong. E.g. using your snippet

Quote:

ldi r16,0b01000000
out 0x07,r16

I perhaps don't entirely disagree with you about certain magic numbers for the bit names. But I am shocked and dismayed by the OUT to a hard register number (versus the name from the chip include file that matches the datasheet name). I'd never get anything done right. Perhaps instead of using variable names, you could just use the memory address of them instead? Wouldn't that be a logical extension?

From the title, "ASM finer points". I have to add that to my oxymoron list. (Listening, js? :twisted: )

You can put lipstick on a pig, but it is still a pig.

I've never met a pig I didn't like, as long as you have some salt and pepper.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Perhaps instead of using variable names, you could just use the memory address of them instead?
haha, indeed, good point.
Good luck keeping track of variables on the stack though.

"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it"

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Where the rubber meets the road is not in the original source, but during debugging. If you have a symbolic debugger that can resolve symbols into numbers and vice versa then you can look at the disassembly and look at the datasheet and verify by inspection that the code is correct.

It has happened to me that I have applied an expression for a particular bit to the wrong register. It is in these cases that establishing the correct mapping is essential.

We never have time to do it right,
but we always have time to do it over

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Pragma wrote:
Sorry if this is too esoteric, (or just plain picky):-)
Both :)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

One of the main reasons that Atmel chose bit numbers rather than bit masks is that some opcodes require bit numbers. Bit numbers can be easily used as bit masks (using 1 <<), while bit masks could not be used as bit numbers.

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

The OP seems to imply that bitmasks are actually first class citizens, while bit numbers are second class. I can not understand why this is so.

Quote:

To increase transparency, the label could be changed to ADSCbit6

But then you could just as well use something like 01000000?

[edit]I "slipped" into C mode below, but... My primary language for AVRs these days is C, but the below argument holds equally well for assembler (which I also am fully capable of using). You are free to skip/discard the struct corollary though.[/edit]

"Transparency" is used in the sense "not having to bother with the exact position of the bit". While writing the code, and to a large extent when reading it, you gain nothing by knowing the exact bit position. OTOH you actually loose clarity (which might be a better choice of wording than "transparency") since you can not keep a backwards translation table from bit masks (or bit numbers for that matter) to bit names.

(As a corollary, I suppose that you also dislike that C structs actually uses symbolic names for member fields. They should rather be referenced numerically by position, perhaps?)

The only AVR special purpose registers that have somewhat "generic" or "anonymous" bit names are the PORT/PIN/DDR registers. But I bet most people here would, as soon as they connect e.g. PORTB pin 2 to an Enable signal of a LCD display module they will do something like

#define LCD_ENABLE 2

so that their code can be more transparent when wiggling that pin, e.g.

PORTB |= 1<<LCD_ENABLE;

rather than

PORTB |= 1<<2;

or even

PORTB |= 0b00000100;

The first is largely self-documenting (if you know your C bit manipulation operators), whilst the two latter definitively needs a comment like

// Raise the Enable signal

to be readable when you no longer have your 2KLOCs of code and the schematics in your short- or long-term memory any more.

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Another example would be a design change. Let's say your code manipulates PORTB.3 at 50 different locations in your code. You do

ldi r16,0x08
out portb,r16

Now someone comes along and changes the hardware to use PORTB.2 instead. Now you have 50 lines of code to change, hopefully you don't miss anything.

If you had done

.define EN 3

ldi r16,EN
out portb,r16

then you change one line of code and no chance of missing a line to change.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

ldi r16,1<<ADSC
out ADCSRA,r16 

IMHO, ADSC should be defined (elsewhere) as 0b01000000 or 1<<0x06


Quote:

sbi ADCSRA,ADSC

IMHO, ADSC should be defined (elsewhere) as 6 or log2(0b0100000)).

On Harvard architectures full address must at least hold x.y.z, where:

    - x as memory type{lock,flash,sram,IO,extIO,EEPROM..} - y as address of storage type {byte/word/page..}
    - z as location within storage type {bit/nibble..}.

And that is how it is made on some compilers (cannot assign pointer from sram to a pointer from flash etc.) Unfortunately avr-gcc does not explicitly use "x" address (it is implicitly passed in function/macro name) and because of byte-oriented C, "z" is set as masks...

Quote:
I have applied an expression for a particular bit to the wrong register.

ADCSRA|=_BV(ACBG);

This error could be found even with only "y" and "z" information.
This one is interesting(ACBG==ADSC):

ADCSRA|=_BV(ACBG)|_BV(ADSC);

No RSTDISBL, no fun!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

kk6gm wrote:
thygate wrote:
well, seeing you write (1<<ADSC) to ADCSRA, i immediately know what you intended to do there, without having to open the datasheet.

You should not be counting bits anyway, as the name in the datasheet (ADSC) corresponds to the definitions in the include file. Why mess with numbers when there are symbols.

Why would i care that ADSC is bit 6 ?


Exactly what I would have replied. The symbolic names convey the important information. The associated numbers are usually unimportant details. When I see code that calls a subroutine do I gain more understanding by seeing the name of the subroutine or its address?

I think we are all generally agreeing here, but I sense a confusion between what I am saying and what the tutorial was saying. The snippet I provided was not my code, but from a tutorial.

I am all for meaningful symbolic names, I just don't understand the value and purpose of performing the bit shift inline with the ldi rather than just defining it in the datasheet. Either way, you must refer to the datsheet.

thygate wrote:
well, seeing you write (1<<ADSC) to ADCSRA, i immediately know what you intended to do there, without having to open the datasheet.

Sorry, I don't buy this. The 1<< provides no additional information, in this case, because for a given system ASDC will always be the same. IOW, there is nothing the bitshift adds that couldn't be accomplished with a strait ldi of a predefined constant.

I'm not saying that this approach would never be useful, for example where the shift length is conditional on a variable.

I don't want to beat this to death. From the comments, it looks like the tutorial example was weak, but as a newbie I was concerned that I was missing something deeper.

Feel free to disagree, I'm keen to learn.

Cheers,

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

I just don't understand the value and purpose of performing the bit shift inline with the ldi rather than just defining it in the datasheet

You seem to have missed the point about opcodes such as SBI and CBI which are used in the form:

SBI SFR, bit_num

(bit_num = 0..7). So Atmel define things such as:

#define SPIF 5

so you can use:

SBIS SPSR, SPIF

If it was defined a 2^5 that is:

#define SPIF 0x20

then sure you could use:

IN r24, SPSR
ANDI r24, SPIF

but how would you use that 0x20 value in SBI, CBI, SBIS, etc?

On the other hand if you are given:

#define SPIF 5

you can easily convert this into 0x20 for use elsewhere with (1<<bit_num) such as:

IN r24, SPSR
ANDI r24, (1<<SPIF)

THAT is why bit numbers, not bit masks are defined. If you don't like then:

#define mySPIF 0x20

and use that (but good luck with SBI, CBI etc!)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

dksmall wrote:
Another example would be a design change. Let's say your code manipulates PORTB.3 at 50 different locations in your code. You do

ldi r16,0x08
out portb,r16

Now someone comes along and changes the hardware to use PORTB.2 instead. Now you have 50 lines of code to change, hopefully you don't miss anything.

If you had done

.define EN 3

ldi r16,EN
out portb,r16

then you change one line of code and no chance of missing a line to change.

No argument here at all. If you refer to my original post:

pragma wrote:

Obviously, defining a constant once eliminates the need, and danger, of changing multiple absolute values as hardware changes, but most of what is said in the tutorial seems to be a weak excuse to show the shift left function.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Pragma wrote:
kk6gm wrote:
thygate wrote:
well, seeing you write (1<<ADSC) to ADCSRA, i immediately know what you intended to do there, without having to open the datasheet.

You should not be counting bits anyway, as the name in the datasheet (ADSC) corresponds to the definitions in the include file. Why mess with numbers when there are symbols.

Why would i care that ADSC is bit 6 ?


Exactly what I would have replied. The symbolic names convey the important information. The associated numbers are usually unimportant details. When I see code that calls a subroutine do I gain more understanding by seeing the name of the subroutine or its address?

....
The 1<< provides no additional information, in this case, because for a given system ASDC will always be the same. IOW, there is nothing the bitshift adds that couldn't be accomplished with a strait ldi of a predefined constant.

Actually, if you get right down to it, it provides the additional information that a single bit is being manipulated. But beyond that, it can certainly be done either way, and I have seen header files that provide both bit numbers and bit masks for the same bit.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I seem to have misunderstood your point, it being about the shift operator rather than using symbol names vs. using numbers.

I personally don't mind the (1<<x) operator, as i find it provides visual reference that makes it somewhat faster to get an overview of a block of code.
Much like you don't see letters, but see words.

UCSR1B = (1<<RXEN1)|(1<<TXEN1)|(1<<RXCIE1); 

VS.

UCSR1B = RXEN1|TXEN1|RXCIE1; 

Although they are both equally readable to me, i actually find the first one to be slightly easier on the eyes in this case.

"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it"

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

I am all for meaningful symbolic names

but initially you wrote
Quote:

I disagree with the above. ADSC is actually LESS transparent in the first example, because its value is defined elsewhere.

which seem to suggest exactly that it is the symbolic name that you are against.

Yes, other parts of the original post suggests that you are also not liking the shift operation.
Yes, there could have been two different defines - one for the bit number and one for the bit mask. Still don't think that the specific bit position number should be spelled out in either of them though, the names should be e.g. ADSC_bitnumber and ADSC_mask in that case.

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

JohanEkdahl wrote:
Quote:

I am all for meaningful symbolic names

but initially you wrote
Quote:

I disagree with the above. ADSC is actually LESS transparent in the first example, because its value is defined elsewhere.

which seem to suggest exactly that it is the symbolic name that you are against.

I see where you are confused, and rightly so. I am not against symbolic names at all. I was merely disagreeing with the tutorial as to what was and what was not transparent. It has been suggested by others in this thread that transparent is a poor choice of word.

Quote:
Yes, other parts of the original post suggests that you are also not liking the shift operation.
Yes, there could have been two different defines - one for the bit number and one for the bit mask. Still don't think that the specific bit position number should be spelled out in either of them though, the names should be e.g. ADSC_bitnumber and ADSC_mask in that case.

Multiple definitions make sense. My objection to the shift operation was that it was, IMHO an unnecessary step, although the overhead was just during assembly.

To me, this is like loading a register with FF and then xor'ing it with FE in order to load the register with 01.

I realize that it is a fine point, but why bother shifting something that has no logical reason to be shifted. It is a bit mask and will only be used in one position for a given system, which is part of its very definition.

I'm not saying that there isn't a place for using an assembler-time directive to set up data and constants; I just don't think this is one of them.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
Multiple definitions make sense.
Only if the bit mask definition is based off the bit position definition.

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Koshchi wrote:
Quote:
Multiple definitions make sense.
Only if the bit mask definition is based off the bit position definition.

Yes! Good point!

Pragma (ex Wet Coaster)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
Pragma (ex Wet Coaster)

Hopefully that should be West...

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

dksmall wrote:

Pragma (ex Wet Coaster)

Hopefully that should be West...

LOL! You've never spent a winter in Vancouver have you?