fix for aarch64 sim tbnz bug
Commit Message
Debugged another gcc testsuite failure, and found that tbnz/tbz are
broken when the bit position to test is greater than 31. There are
two problems. The high bit of the bit position is shifted left by the
wrong amount. And we need to use (uint64_t)1 to get a 64-bit shift
result.
Tested with a gcc C testsuite run. This reduces failures from 2856 to 2710.
Jim
Comments
On Thu, Dec 1, 2016 at 11:49 PM, Jim Wilson wrote:
> Debugged another gcc testsuite failure, and found that tbnz/tbz are
> broken when the bit position to test is greater than 31. There are
> two problems. The high bit of the bit position is shifted left by the
> wrong amount. And we need to use (uint64_t)1 to get a 64-bit shift
> result.
>
> Tested with a gcc C testsuite run. This reduces failures from 2856 to 2710.
can we please start getting tests added to sim too ? using gcc
indirectly to validate the sim is a bit un
-mike
Hi Jim,
> Tested with a gcc C testsuite run. This reduces failures from 2856 to 2710.
Patch approved - please apply.
Just one question:
+ if (aarch64_get_reg_u64 (cpu, rt, NO_SP) & (((uint64_t) 1) << pos))
Would:
+ if (aarch64_get_reg_u64 (cpu, rt, NO_SP) & (1UL << pos))
work as well, or would this break on 32-bit hosts ?
Cheers
Nick
On Dez 02 2016, Nick Clifton <nickc@redhat.com> wrote:
> Would:
>
> + if (aarch64_get_reg_u64 (cpu, rt, NO_SP) & (1UL << pos))
>
> work as well, or would this break on 32-bit hosts ?
+ if ((aarch64_get_reg_u64 (cpu, rt, NO_SP) >> pos) & 1)
would work everywhere.
Andreas.
On Fri, Dec 2, 2016 at 4:03 AM, Nick Clifton <nickc@redhat.com> wrote:
> Just one question:
> + if (aarch64_get_reg_u64 (cpu, rt, NO_SP) & (((uint64_t) 1) << pos))
> Would:
> + if (aarch64_get_reg_u64 (cpu, rt, NO_SP) & (1UL << pos)
> work as well, or would this break on 32-bit hosts ?
I don't think that 1UL works, as long could be 32-bits. It would have
to be 1ULL. But that assumes that long long is 64-bits.
aarch64_get_reg_u64 is defined to return uint64_t, so casting to that
seemed the best choice to me, to keep types consistent.
Jim
sim/aarch64
* simulator.c (tbnz, tbz): Cast 1 to uint64_t before shifting.
(dexTestBranchImmediate): Shift high bit of pos by 5 not 4.
@@ -13353,7 +13353,7 @@ tbnz (sim_cpu *cpu, uint32_t pos, int32_t offset)
unsigned rt = INSTR (4, 0);
TRACE_DECODE (cpu, "emulated at line %d", __LINE__);
- if (aarch64_get_reg_u64 (cpu, rt, NO_SP) & (1 << pos))
+ if (aarch64_get_reg_u64 (cpu, rt, NO_SP) & (((uint64_t) 1) << pos))
aarch64_set_next_PC_by_offset (cpu, offset);
}
@@ -13364,7 +13364,7 @@ tbz (sim_cpu *cpu, uint32_t pos, int32_t offset)
unsigned rt = INSTR (4, 0);
TRACE_DECODE (cpu, "emulated at line %d", __LINE__);
- if (!(aarch64_get_reg_u64 (cpu, rt, NO_SP) & (1 << pos)))
+ if (!(aarch64_get_reg_u64 (cpu, rt, NO_SP) & (((uint64_t) 1) << pos)))
aarch64_set_next_PC_by_offset (cpu, offset);
}
@@ -13407,7 +13407,7 @@ dexTestBranchImmediate (sim_cpu *cpu)
instr[18,5] = simm14 : signed offset counted in words
instr[4,0] = uimm5 */
- uint32_t pos = ((INSTR (31, 31) << 4) | INSTR (23, 19));
+ uint32_t pos = ((INSTR (31, 31) << 5) | INSTR (23, 19));
int32_t offset = simm32 (aarch64_get_instr (cpu), 18, 5) << 2;
NYI_assert (30, 25, 0x1b);