http://cgi.ebay.co.uk/ws/eBayISAPI.dll?ViewItem&item=150973943678&ssPageName =STRK:MESE:IT
-----Original Message----- From: [email protected] [mailto:[email protected]] On Behalf Of Sebastien Bourdeauducq Sent: 08 January 2013 21:05 To: Milkymist developers' list Subject: Re: [Milkymist-devel] [PATCH] Warnings on navre Hi, thanks for the patch! > -parameter IO_SEL_EXT = 2'd0; > -parameter IO_SEL_STACK = 2'd1; > -parameter IO_SEL_SREG = 2'd2; > +localparam IO_SEL_EXT = 2'd0; > +localparam IO_SEL_STACK = 2'd1; > +localparam IO_SEL_SREG = 2'd2; Applied. > case(Rd) > > - default: GPR_Rd = GPR_Rd8; > > 5'd24: GPR_Rd = U[7:0]; > 5'd25: GPR_Rd = U[15:8]; > 5'd26: GPR_Rd = pX[7:0]; > > @@ -130,12 +129,12 @@ always @(*) begin > > 5'd29: GPR_Rd = pY[15:8]; > 5'd30: GPR_Rd = pZ[7:0]; > 5'd31: GPR_Rd = pZ[15:8]; > > + default: GPR_Rd = GPR_Rd8; > > endcase The original code (with "default" at the beginning) is clearer I think, since it keeps the registers addresses in order. > - default: GPR_Rr = GPR_Rr8; > > 5'd24: GPR_Rr = U[7:0]; > 5'd25: GPR_Rr = U[15:8]; > 5'd26: GPR_Rr = pX[7:0]; > > @@ -144,6 +143,7 @@ always @(*) begin > > 5'd29: GPR_Rr = pY[15:8]; > 5'd30: GPR_Rr = pZ[7:0]; > 5'd31: GPR_Rr = pZ[15:8]; > > + default: GPR_Rr = GPR_Rr8; same. > +localparam PC_SEL_NOP = 4'd0; > +localparam PC_SEL_INC = 4'd1; ... > +localparam PC_SEL_EX = 4'd9; Applied. > - next_irq_ack = 8'b0; > > casex(irq) > > 8'bxxxx_xxx1: next_irq_ack = 8'b0000_0001; > 8'bxxxx_xx10: next_irq_ack = 8'b0000_0010; > > @@ -197,6 +196,7 @@ always @(*) begin > > 8'bxx10_0000: next_irq_ack = 8'b0010_0000; > 8'bx100_0000: next_irq_ack = 8'b0100_0000; > 8'b1000_0000: next_irq_ack = 8'b1000_0000; > > + default: next_irq_ack = 8'b0; The original code is perfectly valid and latch-free, synthesizers should not display a warning here. > - PC_ex = 4'b0; > > casex(irq) > > 8'bxxxx_xxx1: PC_ex = 4'h0; > 8'bxxxx_xx10: PC_ex = 4'h1; > > @@ -225,6 +224,7 @@ always @(*) begin > > 8'bxx10_0000: PC_ex = 4'h5; > 8'bx100_0000: PC_ex = 4'h6; > 8'b1000_0000: PC_ex = 4'h7; > > + default: PC_ex = 4'h0; same. > +localparam DMEM_SEL_UNDEFINED = 3'bxxx; ... > +localparam DMEM_SEL_SP_PCH = 4'd11; > +localparam DMEM_SEL_PMEM = 4'd12; Fixed tabulation problems and applied. > +localparam NORMAL = 5'd0; > +localparam RCALL = 5'd1; ... > +localparam RETI4 = 5'd17; Applied. > @@ -803,6 +803,7 @@ always @(*) begin > > 5'b1_0001: dmem_sel = DMEM_SEL_ZPLUS; > 5'b1_0010: dmem_sel = DMEM_SEL_ZMINUS; > 5'b0_0xxx: dmem_sel = DMEM_SEL_ZQ; > > + default: dmem_sel = DMEM_SEL_SP_PCL; What is the purpose of this? only to avoid a "default missing" warning? If so, dmem_sel is set to DMEM_SEL_UNDEFINED at the beginning of the always block (why did you choose DMEM_SEL_SP_PCL?), and then again, this is valid and latch-free. > - 16'b1001_00xx_xxxx_xxxx: begin > - if(pmem_d[3:0] == 4'hf) begin ... > - end else if(pmem_d[3:0] == 4'h0) begin > + 16'b1001_00xx_xxxx_1111: begin ... > + 16'b1001_00xx_xxxx_0000: begin ... Good catch - applied. Sebastien _______________________________________________ http://lists.milkymist.org/listinfo.cgi/devel-milkymist.org IRC: #milkymist@Freenode _______________________________________________ http://lists.milkymist.org/listinfo.cgi/devel-milkymist.org IRC: #milkymist@Freenode
