Thanks Stefan,
Fix is up on the mailing list. I went for the simple solution of just
always initing the tcg variables with tcg_temp_*new. May be a minor
performance penalty but noting noticeable.
Regards,
Peter
On Tue, Jun 5, 2012 at 6:26 AM, Stefan Weil <s...@weilnetz.de> wrote:
> Hi Peter,
>
> this patch (which was applied to QEMU master) breaks debug builds
> (configure --enable-debug). see my comment below.
>
> Regards,
>
> Stefan W.
>
>
>
> Am 01.06.2012 05:23, schrieb Peter A. G. Crosthwaite:
>>
>> Signed-off-by: Peter A. G. Crosthwaite<peter.crosthwa...@petalogix.com>
>>
>> ---
>> changed from v3:
>> simplified tcg local variable usage aqcross branch
>> changed from v2:
>> fixed tcg local variable usage across branch
>> reworked carry logic (with new write_carryi() function)
>> made LOG_DIS show lwx swx properly
>> changed from v1:
>> implemented reservation address checking
>> created new cpu state variable specifically for reservation address/flag
>> state
>>
>> target-microblaze/cpu.c | 1 +
>> target-microblaze/cpu.h | 4 ++
>> target-microblaze/helper.c | 2 +
>> target-microblaze/translate.c | 62
>> +++++++++++++++++++++++++++++++++++++---
>> 4 files changed, 64 insertions(+), 5 deletions(-)
>>
>
> ...
>
>> diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c
>> index a362938..f0ebd59 100644
>> --- a/target-microblaze/translate.c
>> +++ b/target-microblaze/translate.c
>> @@ -162,6 +162,14 @@ static void write_carry(DisasContext *dc, TCGv v)
>> tcg_temp_free(t0);
>> }
>>
>> +static void write_carryi(DisasContext *dc, int carry)
>> +{
>> + TCGv t0 = tcg_temp_new();
>> + tcg_gen_movi_tl(t0, carry ? 1 : 0);
>> + write_carry(dc, t0);
>> + tcg_temp_free(t0);
>> +}
>> +
>> /* True if ALU operand b is a small immediate that may deserve
>> faster treatment. */
>> static inline int dec_alu_op_b_is_small_imm(DisasContext *dc)
>> @@ -948,12 +956,13 @@ static inline void dec_byteswap(DisasContext *dc,
>> TCGv dst, TCGv src, int size)
>> static void dec_load(DisasContext *dc)
>> {
>> TCGv t, *addr;
>> - unsigned int size, rev = 0;
>> + unsigned int size, rev = 0, ex = 0;
>>
>> size = 1<< (dc->opcode& 3);
>>
>> if (!dc->type_b) {
>> rev = (dc->ir>> 9)& 1;
>>
>> + ex = (dc->ir>> 10)& 1;
>> }
>>
>> if (size> 4&& (dc->tb_flags& MSR_EE_FLAG)
>>
>> @@ -963,7 +972,8 @@ static void dec_load(DisasContext *dc)
>> return;
>> }
>>
>> - LOG_DIS("l%d%s%s\n", size, dc->type_b ? "i" : "", rev ? "r" : "");
>> + LOG_DIS("l%d%s%s%s\n", size, dc->type_b ? "i" : "", rev ? "r" : "",
>> + ex ? "x" : "");
>>
>> t_sync_flags(dc);
>> addr = compute_ldst_addr(dc,&t);
>> @@ -1019,6 +1029,17 @@ static void dec_load(DisasContext *dc)
>> }
>> }
>>
>> + /* lwx does not throw unaligned access errors, so force alignment */
>> + if (ex) {
>> + /* Force addr into the temp. */
>> + if (addr !=&t) {
>> + t = tcg_temp_new();
>> + tcg_gen_mov_tl(t, *addr);
>> + addr =&t;
>> + }
>> + tcg_gen_andi_tl(t, t, ~3);
>> + }
>> +
>> /* If we get a fault on a dslot, the jmpstate better be in sync. */
>> sync_jmpstate(dc);
>>
>> @@ -1057,6 +1078,12 @@ static void dec_load(DisasContext *dc)
>> }
>> }
>>
>> + if (ex) { /* lwx */
>> + /* no support for for AXI exclusive so always clear C */
>> + write_carryi(dc, 0);
>> + tcg_gen_st_tl(*addr, cpu_env, offsetof(CPUMBState, res_addr));
>> + }
>> +
>> if (addr ==&t)
>> tcg_temp_free(t);
>> }
>> @@ -1078,12 +1105,14 @@ static void gen_store(DisasContext *dc, TCGv addr,
>> TCGv val,
>>
>> static void dec_store(DisasContext *dc)
>> {
>> - TCGv t, *addr;
>> - unsigned int size, rev = 0;
>> + TCGv t, *addr, swx_addr, r_check = 0;
>
>
> With --enable-debug, TCGv is a struct (not an int),
> therefore r_check = 0 does not work.
>
> swx_addr and r_check are used in the same context.
> Can their declarations be moved to the block where
> they are first used?
>
> If that is not possible, either none or both of these
> two variables should be initialised.
>
>
>> + int swx_skip = 0;
>> + unsigned int size, rev = 0, ex = 0;
>>
>> size = 1<< (dc->opcode& 3);
>> if (!dc->type_b) {
>> rev = (dc->ir>> 9)& 1;
>>
>> + ex = (dc->ir>> 10)& 1;
>> }
>>
>> if (size> 4&& (dc->tb_flags& MSR_EE_FLAG)
>>
>> @@ -1093,12 +1122,30 @@ static void dec_store(DisasContext *dc)
>> return;
>> }
>>
>> - LOG_DIS("s%d%s%s\n", size, dc->type_b ? "i" : "", rev ? "r" : "");
>> + LOG_DIS("s%d%s%s%s\n", size, dc->type_b ? "i" : "", rev ? "r" : "",
>> + ex ? "x" : "");
>> t_sync_flags(dc);
>> /* If we get a fault on a dslot, the jmpstate better be in sync. */
>> sync_jmpstate(dc);
>> addr = compute_ldst_addr(dc,&t);
>>
>> + if (ex) { /* swx */
>> + r_check = tcg_temp_new();
>> + swx_addr = tcg_temp_local_new();
>
>
>
> TCGv r_check = ...;
> TCGv swx_addr = ...;
>
>
>> +
>> + /* Force addr into the swx_addr. */
>> + tcg_gen_mov_tl(swx_addr, *addr);
>> + addr =&swx_addr;
>> + /* swx does not throw unaligned access errors, so force alignment
>> */
>> + tcg_gen_andi_tl(swx_addr, swx_addr, ~3);
>> +
>> + tcg_gen_ld_tl(r_check, cpu_env, offsetof(CPUMBState, res_addr));
>> + write_carryi(dc, 1);
>> + swx_skip = gen_new_label();
>> + tcg_gen_brcond_tl(TCG_COND_NE, r_check, swx_addr, swx_skip);
>> + write_carryi(dc, 0);
>
>
> Add this code from block at end of function?
>
> gen_set_label(swx_skip);
> tcg_temp_free(swx_addr);
> tcg_temp_free(r_check);
>
>> + }
>> +
>> if (rev&& size != 4) {
>>
>> /* Endian reverse the address. t is addr. */
>> switch (size) {
>> @@ -1174,6 +1221,11 @@ static void dec_store(DisasContext *dc)
>> gen_helper_memalign(*addr, tcg_const_tl(dc->rd),
>> tcg_const_tl(1), tcg_const_tl(size - 1));
>> }
>
>
> Move the following conditional block up?
>
>
>> + if (ex) {
>> + gen_set_label(swx_skip);
>> + tcg_temp_free(r_check);
>> + tcg_temp_free(swx_addr);
>> + }
>
>
