|
From: Peter B. <be...@vn...> - 2013-07-15 04:51:01
|
On Thu, 2013-07-11 at 07:33 -0700, Carl E. Love wrote:
> The patch causes the execution flow to take
> the TM failure path as expected. The compiler is generating a branch if not equal to decide if it
> should take the TM path or the failure path. For now, the Valgrind patch just assumes the compiler
> will always generate the branch if not equal instruction to take one of the two paths. Furthermore,
> it is assumed there will always be a failure path. I need to talk with Peter when he gets back about
> the code generated by the compiler to determine if the compiler might generate different code
> sequences.
The compiler is free to and actually does generate either a beq or a bne
depending on the circumstances. Specifically, the code it can generate
can look like:
tbegin.
...
beq <failure handler>
// fall-through success handler
or
tbegin.
....
bne <success handler>
// fall-through failure handler
Note that the "...." above denotes possible instructions that
might be placed between the tbegin. and the conditional branch,
so you cannot assume the conditional branch immediately follows
the tbegin. Luckily, you don't need to look at the branch at
all. You are only looking at it to compute the address of
the failure handler, but that is not correct. The address of
the failure handler is stored in the TFHAR register and the
tbegin. initializes it to CIA+4 (ie, the next instruction).
> + case 0x80: // 128
> + DIP("mfspr r%u (TFHAR)\n", rD_addr);
> + putIReg( rD_addr, getGST( PPC_GST_TFHAR) );
> + break;
> + case 0x81: // 129
> + DIP("mfspr r%u (TFIAR)\n", rD_addr);
> + putIReg( rD_addr, getGST( PPC_GST_TFIAR) );
> + break;
> + case 0x82: // 130
> + DIP("mfspr r%u (TEXASR)\n", rD_addr);
> + putIReg( rD_addr, getGST( PPC_GST_TEXASR) );
> + break;
Note that the texasr is a 64-bit register in both 32-bit
and 64-bit modes. In 32-bit mode, the "mfspr TEXASR,rX"
should just place the lower 32-bits of the texasr into
rX. Is that what this code ends up doing?
You are also missing the "mfspr TEXASRU,rX", which is
used by 32-bit code to get the upper 32-bits of the
texasr into rX.
> + case 0x80: // 128
> + DIP("mtspr r%u (TFHAR)\n", rS_addr);
> + putGST( PPC_GST_TFHAR, mkexpr(rS) );
> + break;
> + case 0x81: // 129
> + DIP("mtspr r%u (TFIAR)\n", rS_addr);
> + putGST( PPC_GST_TFIAR, mkexpr(rS) );
> + break;
> + case 0x82: // 130
> + DIP("mtspr r%u (TEXASR)\n", rS_addr);
> + putGST( PPC_GST_TEXASR, mkexpr(rS) );
> + break;
Ditto here.
> + DIP("tbegin. %d\n", R);
> + if (opc1_next == 0x10) { // conditional branch
[snip]
As mentional above, there is no need to look at the following
branch, so your "if (opc1_next == 0x10)" test can be removed.
Just unconditionally execute the then clause code and remove
the error code in the else clause.
> + /* Get the address of the failure handler from the conditional
> + * branch in the next instruction location.
> + */
> + if ( flag_AA )
> + failure_tgt = mkSzAddr( ty, extend_s_16to64( BD_u16 ) );
> + else
> + failure_tgt = mkSzAddr( ty, guest_CIA_curr_instr +
> + (Long)extend_s_16to64( BD_u16 ) );
To be pedantic, the address of the failure handler is equal to
the address that is in the TFHAR register, not the address from
the conditional branch. That is especially true given that the
compiler is free to generate either:
tbegin.
...
beq <failure handler>
// fall-through success handler
or
tbegin.
....
bne <success handler>
// fall-through failure handler
Remember that tbegin. initializes the TFHAR to CIA+4, so you
just have to set failure_tgt to the address of nextInstr.
> + /* Set the CR0 field to indicate the tbegin failed. Then let
> + * the code do the branch to the failure path.
> + *
> + * 000 || 0 Transaction initiation successful,
> + * unnested (Transaction state of
> + * Non-transactional prior to tbegin.)
> + * 010 || 0 Transaction initiation successful, nested
> + * (Transaction state of Transactional
> + * prior to tbegin.)
> + * 001 || 0 Transaction initiation unsuccessful,
> + * (Transaction state of Suspended prior
> + * to tbegin.)
> + */
> + if (mode64)
> + /* 0x0010 takes transactional path */
> + /* 0x0000 takes the failure path */
> + set_CR0(mkU64(0x0000));
> + else
> + set_CR0(mkU32(0x0000));
Your comment is correct, but you are incorrectly clearing cr0,
which signifies the tbegin. succeeded. You need to initialize
it to 0x2.
Stylistically, using "0x0000" looks like you're trying to stuff
4 4-bit nibbles into cr0, but a cr register can only hold 4-bits
(ie, 1 nibble). You could use 0x2 or 0b0010, which both look more
like just 1 nibble of data. Then again, I don't know the valgrind
code formatting rules and maybe that is how things are written?
If so, just ignore this comment of mine.
Peter
|