According to AVR manuals 16 bit timer registers share a temp register which
requires a specific sequence for reading/writing: "To do a 16-bit write,
the high byte must be written before the low byte. For a 16-bit read, the
low byte must be read before the high byte." - from atmega328p 15.3
Accessing 16-bit Registers. I'm trying to patch the AVR code generator to
respect this sequence but I would appreciate some review of my attempt and
suggestions on how to improve it.
Attached please find a patch where I added code to avr/cgcpu methods
a_load_reg_ref and g_concatcopy. I try to identify 16 bit io registers by
checking whether the offset of the reference falls within 32 to srambase-1
and the size is OS_16/OS_S16 (or length=2). With this patch the compiler
generates (what I assume is) correct code for the following test cases
which should write to register 0x87 then 0x86 (-Wpatmega328p, so timer1 is
16-bit, -O2):
var x: uint16 = 1023;
ICR1 := x;
lds r0, 0x0101
sts 0x0087, r0
lds r0, 0x0100
sts 0x0086, r0
ICR1 := 1234;
ldi r18, 0xD2
ldi r19, 0x04
sts 0x0087, r19
sts 0x0086, r18
ICR1 := OCR1A; // should first read 0x88 then 0x89 and then write 0x87, 0x86
lds r19, 0x0088
lds r18, 0x0089
sts 0x0087, r18
sts 0x0086, r19
x := OCR1A; // read from io reg should be low byte first
lds r0, 0x0088
sts 0x0100, r0
lds r0, 0x0089
sts 0x0101, r0
Any further test cases I should consider (also other code which should not
trigger this modification) and ways to improve the patch would be
appreciated.
Regards,
Christo
Index: avr/cgcpu.pas
===================================================================
--- avr/cgcpu.pas (revision 40380)
+++ avr/cgcpu.pas (working copy)
@@ -1345,21 +1345,38 @@
end;
if not conv_done then
begin
- for i:=1 to tcgsize2size[fromsize] do
+ // CC
+ // Write to 16 bit ioreg, first high byte then low byte
+ // sequence required for 16 bit timer registers
+ // See e.g. atmega328p manual para 15.3 Accessing 16 bit registers
+ if (fromsize in [OS_16, OS_S16]) and QuickRef and (href.offset > 31)
+ and (href.offset < cpuinfo.embedded_controllers[current_settings.controllertype].srambase) then
begin
- if not(QuickRef) and (i<tcgsize2size[fromsize]) then
- href.addressmode:=AM_POSTINCREMENT
- else
- href.addressmode:=AM_UNCHANGED;
-
+ tmpreg:=GetNextReg(reg);
+ href.addressmode:=AM_UNCHANGED;
+ inc(href.offset);
+ list.concat(taicpu.op_ref_reg(GetStore(href),href,tmpreg));
+ dec(href.offset);
list.concat(taicpu.op_ref_reg(GetStore(href),href,reg));
+ end
+ else
+ begin
+ for i:=1 to tcgsize2size[fromsize] do
+ begin
+ if not(QuickRef) and (i<tcgsize2size[fromsize]) then
+ href.addressmode:=AM_POSTINCREMENT
+ else
+ href.addressmode:=AM_UNCHANGED;
- if QuickRef then
- inc(href.offset);
+ list.concat(taicpu.op_ref_reg(GetStore(href),href,reg));
- { check if we are not in the last iteration to avoid an internalerror in GetNextReg }
- if i<tcgsize2size[fromsize] then
- reg:=GetNextReg(reg);
+ if QuickRef then
+ inc(href.offset);
+
+ { check if we are not in the last iteration to avoid an internalerror in GetNextReg }
+ if i<tcgsize2size[fromsize] then
+ reg:=GetNextReg(reg);
+ end;
end;
end;
@@ -2269,42 +2286,93 @@
dstref:=dest;
end;
- for i:=1 to len do
- begin
- if not(SrcQuickRef) and (i<len) then
- srcref.addressmode:=AM_POSTINCREMENT
- else
- srcref.addressmode:=AM_UNCHANGED;
+ // CC
+ // If dest is an ioreg (31 < offset < srambase) and size = 16 bit then
+ // load high byte first, then low byte
+ if (len = 2) and DestQuickRef
+ and (dest.offset > 31)
+ and (dest.offset < cpuinfo.embedded_controllers[current_settings.controllertype].srambase) then
+ begin
+ // If src is also a 16 bit ioreg then read low byte then high byte
+ if SrcQuickRef and (srcref.offset > 31)
+ and (srcref.offset < cpuinfo.embedded_controllers[current_settings.controllertype].srambase) then
+ begin
+ // First read source into temp registers
+ tmpreg:=getintregister(list, OS_16);
+ list.concat(taicpu.op_reg_ref(GetLoad(srcref),tmpreg,srcref));
+ inc(srcref.offset);
+ tmpreg2:=GetNextReg(tmpreg);
+ list.concat(taicpu.op_reg_ref(GetLoad(srcref),tmpreg2,srcref));
- if not(DestQuickRef) and (i<len) then
- dstref.addressmode:=AM_POSTINCREMENT
- else
- dstref.addressmode:=AM_UNCHANGED;
+ // then move temp registers to dest in reverse order
+ inc(dstref.offset);
+ list.concat(taicpu.op_ref_reg(GetStore(dstref),dstref,tmpreg2));
+ dec(dstref.offset);
+ list.concat(taicpu.op_ref_reg(GetStore(dstref),dstref,tmpreg));
+ end
+ else
+ begin
+ srcref.addressmode:=AM_UNCHANGED;
+ inc(srcref.offset);
+ dstref.addressmode:=AM_UNCHANGED;
+ inc(dstref.offset);
- cg.getcpuregister(list,NR_R0);
- list.concat(taicpu.op_reg_ref(GetLoad(srcref),NR_R0,srcref));
- list.concat(taicpu.op_ref_reg(GetStore(dstref),dstref,NR_R0));
- cg.ungetcpuregister(list,NR_R0);
+ cg.getcpuregister(list,NR_R0);
+ list.concat(taicpu.op_reg_ref(GetLoad(srcref),NR_R0,srcref));
+ list.concat(taicpu.op_ref_reg(GetStore(dstref),dstref,NR_R0));
+ cg.ungetcpuregister(list,NR_R0);
- if SrcQuickRef then
- inc(srcref.offset);
- if DestQuickRef then
- inc(dstref.offset);
- end;
- if not(SrcQuickRef) then
- begin
- ungetcpuregister(list,srcref.base);
- ungetcpuregister(list,TRegister(ord(srcref.base)+1));
- end;
- if not(DestQuickRef) then
- begin
- ungetcpuregister(list,dstref.base);
- ungetcpuregister(list,TRegister(ord(dstref.base)+1));
- end;
- end;
- end;
+ if not(SrcQuickRef) then
+ srcref.addressmode:=AM_POSTINCREMENT
+ else
+ srcref.addressmode:=AM_UNCHANGED;
+ dec(srcref.offset);
+ dec(dstref.offset);
+ cg.getcpuregister(list,NR_R0);
+ list.concat(taicpu.op_reg_ref(GetLoad(srcref),NR_R0,srcref));
+ list.concat(taicpu.op_ref_reg(GetStore(dstref),dstref,NR_R0));
+ cg.ungetcpuregister(list,NR_R0);
+ end;
+ end
+ else
+ for i:=1 to len do
+ begin
+ if not(SrcQuickRef) and (i<len) then
+ srcref.addressmode:=AM_POSTINCREMENT
+ else
+ srcref.addressmode:=AM_UNCHANGED;
+
+ if not(DestQuickRef) and (i<len) then
+ dstref.addressmode:=AM_POSTINCREMENT
+ else
+ dstref.addressmode:=AM_UNCHANGED;
+
+ cg.getcpuregister(list,NR_R0);
+ list.concat(taicpu.op_reg_ref(GetLoad(srcref),NR_R0,srcref));
+ list.concat(taicpu.op_ref_reg(GetStore(dstref),dstref,NR_R0));
+ cg.ungetcpuregister(list,NR_R0);
+
+ if SrcQuickRef then
+ inc(srcref.offset);
+ if DestQuickRef then
+ inc(dstref.offset);
+ end;
+ if not(SrcQuickRef) then
+ begin
+ ungetcpuregister(list,srcref.base);
+ ungetcpuregister(list,TRegister(ord(srcref.base)+1));
+ end;
+ if not(DestQuickRef) then
+ begin
+ ungetcpuregister(list,dstref.base);
+ ungetcpuregister(list,TRegister(ord(dstref.base)+1));
+ end;
+ end;
+ end;
+
+
procedure tcgavr.g_overflowCheck(list : TAsmList;const l : tlocation;def : tdef);
var
hl : tasmlabel;
_______________________________________________
fpc-devel maillist - [email protected]
http://lists.freepascal.org/cgi-bin/mailman/listinfo/fpc-devel
