| Author |
Message |
|
|
 Posted: Nov 05, 2011 - 12:55 PM |
|
Joined: Sep 17, 2006
Posts: 37
Location: Czech Republic
|
|
Hi all : )
I'm looking how to read CARRY flag from SREG after certain operation - LEFT SHIFT.
Sample code is following:
Code:
char carry;
char data;
data = data<<1;
// There I need to read bit that was shifted out into carry
It is clear to me this is job for inline ASM, but I never work with ASM in C.
Can you give me some advice?
David |
|
|
| |
|
|
|
|
|
|
Posted: Nov 05, 2011 - 01:16 PM |
|
Joined: Feb 12, 2005
Posts: 16536
Location: Wormshill, England
|
|
Do not even dream of accessing the Carry flag in C. It is not guaranteed to be valid in C.
If you want to know whether a carry is shifted out, you simply:
Code:
char data;
// just check bit 7 before the shift.
char carry = (data & 0x80) != 0;
data = data<<1;
David. |
|
|
| |
|
|
|
|
|
|
Posted: Nov 05, 2011 - 01:40 PM |
|
Joined: Dec 16, 2005
Posts: 3094
Location: Bratislava, Slovakia
|
|
David's reply covers the vast majority of possible uses, but if you believe you need better and you tell us why do you think you need the carry bit, we might come up with alternative solutions.
Jan |
|
|
| |
|
|
|
|
|
|
Posted: Nov 05, 2011 - 02:05 PM |
|
Joined: Sep 17, 2006
Posts: 37
Location: Czech Republic
|
|
Generally, I'm trying to write MFM coder as simple as it is possible. Input is byte array, output is MFM-coded stream.
If it helps you, there is whole procedure - I'm not saying that is the best solution, but until now i haven't wrote better solution.
Base idea, if I understood it OK, is that data bits (x,y,z,...) are coded to (x,x NOR y, y NOR z, z, z NOR .......)
So I came with this solution. David, your code will work of course, but I think variant with use of CARRY flag will save some instructions.
Code:
void FDDIO_encodeMfm() {
char carry = 0;
char prvek = FDD_sector[0];
prvek = (prvek << 1);
//!!! carry = ?; !!!
// Data x
FDD_sectorMfm[0] |= carry;
for(int i = 0; i < 658;i++) {
prvek = FDD_sector[i];
for(c = 0; c < 9; c++) {
// CLOCK x NOR y
FDD_sectorMfm[i] = (short) (FDD_sectorMfm[i] << 1);
FDD_sectorMfm[i] |= carry;
if(c==8 && i!= (658-1)) prvek = FDD_sector[i+1];
prvek = (prvek<<1);
//!!! carry = ?; !!!
FDD_sectorMfm[i] |= carry;
FDD_sectorMfm[i] ^= (1<<0);
// DATA y
if(c < 8) {
FDD_sectorMfm[i] = (FDD_sectorMfm[i] << 1);
FDD_sectorMfm[i] |= carry; // bit 14 16bit slova, 15. je pro ulozeni clocku ktery uz souvisi s dalsim prvkem
}
}
}
}
|
|
|
| |
|
|
|
|
|
|
Posted: Nov 05, 2011 - 02:07 PM |
|
Joined: Sep 17, 2006
Posts: 37
Location: Czech Republic
|
|
Forgotten global defs:
short FDD_sectorMfm[658];
char FDD_sector[512];
.. I have tested almost the same code on PC (java) and it works OK.. |
_________________
David Sedláček
|
| |
|
|
|
|
|
|
Posted: Nov 05, 2011 - 02:21 PM |
|
Joined: Feb 12, 2005
Posts: 16536
Location: Wormshill, England
|
|
You never assign carry apart from initialising to zero.
I should just write the algorithm to be accurate. Do not worry about the efficiency.
Once you are working, by all means optimise it.
I notice that you have multiple array references. Some compilers may spot this. OTOH, you might just as well introduce a temporary variable.
Note that you can equally well use a 16-bit int. Then the 'carry' is in bit 8.
David. |
|
|
| |
|
|
|
|
|
|
Posted: Nov 05, 2011 - 04:26 PM |
|
Joined: Nov 17, 2004
Posts: 13949
Location: Vancouver, BC
|
|
|
Code:
prvek = (prvek<<1);
carry = SREG & (1<<7) != 0;
This could give you the current state of the carry bit, but you must realize what "current" means. It is certainly not the state of the carry bit immediately after the shift since, at the very least, the asignment to prvek has happened since then. There could also be several opcodes run in preparation of reading SREG that might have changed its value. And worst case, an interrupt might have occurred in between which means hundreds or even thousands of opcodes could have been run.
Davids solution of using a 16 bit int sounds easiest to me (however, you need to change the declaration of prvek to unsigned char, which is probably advisable anyways):
Code:
unsigned int temp = prvek << 1;;
prvek = temp;
carry = temp >> 8;
|
_________________
Regards,
Steve A.
The Board helps those that help themselves.
|
| |
|
|
|
|
|
|
Posted: Nov 05, 2011 - 04:49 PM |
|
Joined: Jul 23, 2001
Posts: 2467
Location: Osnabrueck, Germany
|
|
|
Quote:
And worst case, an interrupt might have occurred in between which means hundreds or even thousands of opcodes could have been run.
If you use a compiler that does not preserve SREG in interrupts, then you should consider to use a different one. 
(of course the central point "reading Carry from SREG in C is pointless" is true nevertheless) |
_________________
Stefan Ernst
|
| |
|
|
|
|
|
|
Posted: Nov 06, 2011 - 09:21 AM |
|
Joined: Oct 05, 2006
Posts: 2296
Location: Poland
|
|
Well, you can read SREG as any other IO register, but this will not bring you any closer with your
Quote:
I'm trying to write MFM coder as simple as it is possible (..)with GCC
idea because GCC does not understand SREG flags and their brbx functionality. |
_________________
No RSTDISBL, no fun!
|
| |
|
|
|
|
|
|
Posted: Nov 06, 2011 - 05:15 PM |
|
Joined: Sep 17, 2006
Posts: 37
Location: Czech Republic
|
|
Hey guys I dont want to work with 16bit integer unless it is not efficient. There must be way to do it in inline ASM this is what I'm asking for : ).
I'm not saying it must be only SREG bit saving line, but what if whole left shift and then SREG bit save section, or more, will be in inline ASM?
Koshchi it is as you wrote - there is some operation(s) after LSHIFT and it is not guaranteed that in CARRY will be correct val.
Again, see this in pseudocode and compare efficiency:
Code:
prvek <<= 1;
prvekMfm |= (SREG&1);
... which I need to convert into inline ASM or generic way:
Code:
carry = (prvek >>7);
prvek <<= 1;
prvekMfm |= carry;
  |
|
|
| |
|
|
|
|
|
|
Posted: Nov 06, 2011 - 05:19 PM |
|
Joined: Mar 27, 2002
Posts: 18746
Location: Lund, Sweden
|
|
| Have you actually looked at what code the compiler generates for an implementation in C? Do that, and then tell what needs further optimization. |
|
|
| |
|
|
|
|
|
|
Posted: Nov 06, 2011 - 05:25 PM |
|
Joined: Jul 23, 2001
Posts: 2467
Location: Osnabrueck, Germany
|
|
|
Quote:
which I need to convert into inline ASM
Why you "need to"? What is wrong with a C solution? With all the other stuff in that function do you really think a few cycles matter? |
_________________
Stefan Ernst
|
| |
|
|
|
|
|
|
Posted: Nov 06, 2011 - 07:16 PM |
|
Joined: Feb 12, 2005
Posts: 16536
Location: Wormshill, England
|
|
You do not need to do all calculations in 16-bit. Simply do a cast whenever you want to 'find' the carry.
Forget about efficiency. Get your algorithm working correctly.
Quite honestly, the C compiler will translate straightforward code very well. Only if it needs extra performance do you ever look at the ASM.
A human can often spot the odd peephole optimisation that will make a significant improvement. But believe me, the Compiler has done most of them already.
The main advantage of the 'tweak generated code' approach is that you have already got a working algorithm. So your testing regime only has to compare against the original.
David. |
|
|
| |
|
|
|
|
|
|
Posted: Nov 06, 2011 - 07:36 PM |
|
Joined: Sep 17, 2006
Posts: 37
Location: Czech Republic
|
|
I need this it running as fast as it is possible. There is working AVR 8bit algorithm.
Code:
void FDDIO_encodeMfm() {
unsigned int prvekMfm = FDD_sectorMfm[0];
unsigned char prvek = FDD_sector[0];
unsigned char carry = 0;
prvek <<= 1;
carry = ((prvek&128)>>7);
// Data x
prvekMfm |= carry;
for(int i = 0; i < 524;i++) {
prvek = FDD_sector[i];
prvekMfm = FDD_sectorMfm[i];
for(c = 0; c < 9; c++) {
// CLOCK x NOR y
prvekMfm <<= 1;
prvekMfm |= (uint8_t) carry;
if((c==8) && (i < (524-1))) {
prvek = FDD_sector[i+1];
}
carry = ((prvek&128)>>7);
prvek <<= 1;
prvekMfm |= carry;
prvekMfm ^= (1<<0);
// DATA y
if(c < 8) {
prvekMfm <<= 1;
prvekMfm |= carry; // bit 14 16bit slova, 15. je pro ulozeni clocku ktery uz souvisi s dalsim prvkem
}
}
FDD_sectorMfm[i] = prvekMfm;
}
}
It counts about 24ops to code 1 data bit with -O3... Data bits are streamed from FDD_sector[] to FDD_sectorMfm[]. I believe there is faster way. |
|
|
| |
|
|
|
|
|
|
Posted: Nov 06, 2011 - 08:47 PM |
|
Joined: Nov 17, 2004
Posts: 13949
Location: Vancouver, BC
|
|
|
Code:
unsigned int prvekMfm = FDD_sectorMfm[0];
unsigned char prvek = FDD_sector[0];
Exactly how do you think you are going to use arrays that have no elements?
Code:
carry = ((prvek&128)>>7);
You say that you want to be as fast as possible, then you choose the most inefficient method suggested in this thread. Besides that, you get the bit after that bit has already been lost. |
_________________
Regards,
Steve A.
The Board helps those that help themselves.
|
| |
|
|
|
|
|
|
Posted: Nov 06, 2011 - 08:50 PM |
|
Joined: Jul 23, 2001
Posts: 2467
Location: Osnabrueck, Germany
|
|
|
Koshchi wrote:
Code:
unsigned int prvekMfm = FDD_sectorMfm[0];
unsigned char prvek = FDD_sector[0];
Exactly how do you think you are going to use arrays that have no elements?
Steve, you should rethink that. |
_________________
Stefan Ernst
|
| |
|
|
|
|
|
|
Posted: Nov 06, 2011 - 08:51 PM |
|
Joined: Jul 18, 2005
Posts: 62899
Location: (using avr-gcc in) Finchingfield, Essex, England
|
|
|
Quote:
Exactly how do you think you are going to use arrays that have no elements?
Steve? he's just assigning element 0 from those two global arrays to each of prvekMfm and prvek surely?
BTW:
Code:
carry = ((prvek&128)>>7);
Haven't checked but what does:
Code:
carry = !!(prvek&128);
yield? |
_________________
|
| |
|
|
|
|
|
|
Posted: Nov 06, 2011 - 08:59 PM |
|
Joined: Jul 23, 2001
Posts: 2467
Location: Osnabrueck, Germany
|
|
|
clawson wrote:
Code:
carry = ((prvek&128)>>7);
Haven't checked but what does:
Code:
carry = !!(prvek&128);
yield?
gcc supports C99, so also worth a check is:
Code:
carry = (_Bool)(prvek&128);
|
_________________
Stefan Ernst
|
| |
|
|
|
|
|
|
Posted: Nov 06, 2011 - 09:01 PM |
|
Joined: Feb 12, 2005
Posts: 16536
Location: Wormshill, England
|
|
I know nothing about MFM. But I am sure there will be algorithms in the public domain.
I would also guess that the whole operation can be done with one or two XORs.
You just need to ask Uncle Google.
I have not looked at the generated ASM. But most compilers will do this in about three instructions.
Code:
if (val & 128) carry = 1;
else carry = 0;
Personally, I think you have written your algorithm fairly clearly (for the compiler). I have made no attempt to understand it myself (human). I just guess that there are better algorithms.
David. |
|
|
| |
|
|
|
|
|
|
Posted: Nov 06, 2011 - 09:30 PM |
|
Joined: Sep 17, 2006
Posts: 37
Location: Czech Republic
|
|
I have tried all variants mentioned there but
carry = ((prvek&128)>>7); is really the fastest.
Code:
47: carry = ((prvek&128)>>7);
MOV R26,R18 Copy register
LSL R26 Logical Shift Left
MOV R19,R26 Copy register
ROL R19 Rotate Left Through Carry
CLR R19 Clear Register
ROL R19 Rotate Left Through Carry
In short it seems in 6 ops R19 contains CARRY, which will be ORed with desired var later (7ops). What I want is to OR this var straight with CARRY (1op)
>>> the same as "ROL" op works.
I believe this is temporary solution until I find how to save CARRY straight to register with desired var. In better some GCC/ASM geek there will tell me how to do it : )
Unfortunatelly MFM coding is very obsolete and was realized very often only by hardware controllers, so there are not any codes nor avr sample codes. Anyway it is really old thing and is described in datasheets from ~70's.... |
|
|
| |
|
|
|
|
|
FAQ
Search
Register
Log in to check your private messages
Log in
Maximize
RSS
