* [PATCH] aarch64 sim big-endian support @ 2016-06-03 2:36 Jim Wilson 2016-06-10 16:24 ` Jim Wilson 2016-06-10 18:06 ` Mike Frysinger 0 siblings, 2 replies; 6+ messages in thread From: Jim Wilson @ 2016-06-03 2:36 UTC (permalink / raw) To: gdb-patches [-- Attachment #1: Type: text/plain, Size: 714 bytes --] On aarch64, code is always little-endian, even when compiling big-endian, so we need to force little-endian when reading instructions. Running the gcc C language testsuite, I get for an aarch64-elf target # of expected passes 35433 # of unexpected failures 254 # of unsupported tests 131 and for an aarch64_be-elf target with the attached patch I get # of expected passes 35200 # of unexpected failures 487 # of unsupported tests 131 so this simple patch gets us most of the way there. I haven't tried looking at the other problems yet. I also have a dejagnu patch I wrote to make this work, which I will be submitting to the dejagnu team shortly. Jim [-- Attachment #2: gdb-sim-aarch64.patch --] [-- Type: text/x-patch, Size: 977 bytes --] 2016-06-02 Jim Wilson <jim.wilson@linaro.org> sim/aarch64/ * simulator.c (aarch64_step): New var saved_target_byte_order. Force byte order to BFD_ENDIAN_LITTLE before pc read, then restore saved value. diff --git a/sim/aarch64/simulator.c b/sim/aarch64/simulator.c index 88cb03d..5a1814c 100644 --- a/sim/aarch64/simulator.c +++ b/sim/aarch64/simulator.c @@ -14078,12 +14078,18 @@ static bfd_boolean aarch64_step (sim_cpu *cpu) { uint64_t pc = aarch64_get_PC (cpu); + enum bfd_endian saved_target_byte_order; if (pc == TOP_LEVEL_RETURN_PC) return FALSE; aarch64_set_next_PC (cpu, pc + 4); + + /* Code is always little-endian. */ + saved_target_byte_order = current_target_byte_order; + current_target_byte_order = BFD_ENDIAN_LITTLE; aarch64_get_instr (cpu) = aarch64_get_mem_u32 (cpu, pc); + current_target_byte_order = saved_target_byte_order; TRACE_INSN (cpu, " pc = %" PRIx64 " instr = %08x", pc, aarch64_get_instr (cpu)); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] aarch64 sim big-endian support 2016-06-03 2:36 [PATCH] aarch64 sim big-endian support Jim Wilson @ 2016-06-10 16:24 ` Jim Wilson 2016-06-13 12:38 ` Nick Clifton 2016-06-10 18:06 ` Mike Frysinger 1 sibling, 1 reply; 6+ messages in thread From: Jim Wilson @ 2016-06-10 16:24 UTC (permalink / raw) To: gdb-patches; +Cc: Nick Clifton ping for the attachment, see https://sourceware.org/ml/gdb-patches/2016-06/msg00046.html On Thu, Jun 2, 2016 at 7:36 PM, Jim Wilson <jim.wilson@linaro.org> wrote: > On aarch64, code is always little-endian, even when compiling > big-endian, so we need to force little-endian when reading > instructions. > > Running the gcc C language testsuite, I get for an aarch64-elf target > > # of expected passes 35433 > # of unexpected failures 254 > # of unsupported tests 131 > > and for an aarch64_be-elf target with the attached patch I get > > # of expected passes 35200 > # of unexpected failures 487 > # of unsupported tests 131 > > so this simple patch gets us most of the way there. I haven't tried > looking at the other problems yet. > > I also have a dejagnu patch I wrote to make this work, which I will be > submitting to the dejagnu team shortly. > > Jim ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] aarch64 sim big-endian support 2016-06-10 16:24 ` Jim Wilson @ 2016-06-13 12:38 ` Nick Clifton 2016-06-30 1:33 ` Jim Wilson 0 siblings, 1 reply; 6+ messages in thread From: Nick Clifton @ 2016-06-13 12:38 UTC (permalink / raw) To: Jim Wilson, gdb-patches [-- Attachment #1: Type: text/plain, Size: 467 bytes --] Hi Jim, > ping oops - sorry. > for the attachment, see > https://sourceware.org/ml/gdb-patches/2016-06/msg00046.html Did you see Mike Frysinger's comment about not changing global variables ? I think that I agree with this comment, although I could not find the raw opcode reading functions to which he was referring, (unless he meant sim_core_read_buffer), so would you mind trying out this variation of your patch to see if it works instead ? Cheers Nick [-- Attachment #2: aarch64-sim.patch --] [-- Type: text/x-patch, Size: 1910 bytes --] diff --git a/sim/aarch64/memory.c b/sim/aarch64/memory.c index 50f4837..f7eea22 100644 --- a/sim/aarch64/memory.c +++ b/sim/aarch64/memory.c @@ -78,6 +78,20 @@ FETCH_FUNC32 (int32_t, int16_t, s16, 2) FETCH_FUNC32 (uint32_t, uint8_t, u8, 1) FETCH_FUNC32 (int32_t, int8_t, s8, 1) +/* Specialized version of aarch64_get_mem_u32 for fetching instructions + which are always held in little endian order even on big-endian + configured targets. */ + +uint32_t +aarch64_get_instruction (sim_cpu *cpu, uint64_t address) +{ + uint32_t val = aarch64_get_mem_u32 (cpu, address); + + if (CURRENT_TARGET_BYTE_ORDER == BFD_ENDIAN_BIG) + val = _SWAP_4 (val); + return val; +} + void aarch64_get_mem_long_double (sim_cpu *cpu, uint64_t address, FRegister *a) { diff --git a/sim/aarch64/memory.h b/sim/aarch64/memory.h index 3f63973..3559245 100644 --- a/sim/aarch64/memory.h +++ b/sim/aarch64/memory.h @@ -37,6 +37,7 @@ extern uint32_t aarch64_get_mem_u8 (sim_cpu *, uint64_t); extern int32_t aarch64_get_mem_s8 (sim_cpu *, uint64_t); extern void aarch64_get_mem_blk (sim_cpu *, uint64_t, char *, unsigned); extern const char * aarch64_get_mem_ptr (sim_cpu *, uint64_t); +extern uint32_t aarch64_get_instruction (sim_cpu *, uint64_t); extern void aarch64_set_mem_long_double (sim_cpu *, uint64_t, FRegister); extern void aarch64_set_mem_u64 (sim_cpu *, uint64_t, uint64_t); diff --git a/sim/aarch64/simulator.c b/sim/aarch64/simulator.c index 88cb03d..c70fd96 100644 --- a/sim/aarch64/simulator.c +++ b/sim/aarch64/simulator.c @@ -14083,7 +14083,7 @@ aarch64_step (sim_cpu *cpu) return FALSE; aarch64_set_next_PC (cpu, pc + 4); - aarch64_get_instr (cpu) = aarch64_get_mem_u32 (cpu, pc); + aarch64_get_instr (cpu) = aarch64_get_instruction (cpu, pc); TRACE_INSN (cpu, " pc = %" PRIx64 " instr = %08x", pc, aarch64_get_instr (cpu)); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] aarch64 sim big-endian support 2016-06-13 12:38 ` Nick Clifton @ 2016-06-30 1:33 ` Jim Wilson 2016-06-30 8:13 ` Nick Clifton 0 siblings, 1 reply; 6+ messages in thread From: Jim Wilson @ 2016-06-30 1:33 UTC (permalink / raw) To: Nick Clifton; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 3473 bytes --] On Mon, Jun 13, 2016 at 5:38 AM, Nick Clifton <nickc@redhat.com> wrote: > I think that I agree with this comment, although I could not find > the raw opcode reading functions to which he was referring, (unless > he meant sim_core_read_buffer), so would you mind trying out this > variation of your patch to see if it works instead ? I finally got back to this. I don't see any raw read function other than sim_core_read_buffer either. A raw read is not quite what I want, as I need a little-endian to host translation, but I can call endian_le2h_4 to do the swap after the raw read. The interface is a little awkward, as sim_core_read_buffer stores into a buffer instead of returning a pointer, so I need to store the instruction, and then read it back out again, swap it, and store it back again. An alternative solution might be to make a copy of sim-n-core.h, call it sim-n-core-le.h, and then change all of the T2H_M/H2T_M calls into LE2H_M/H2LE_M calls, along with a few other minor changes to complete the conversion. We can then call sim_core_read_le_aligned_N instead of sim_core_read_aligned_N for the instruction loads. Note that big-endian aarch64 is not the only target with this problem. big-endian ARMv7-A works the same way, and if we had an IA-64 simulator, it would work the same way too. So there are other potential users of these functions. This is maybe a little overkill though for now, as we don't need the unaligned and misaligned read functions for aarch64/armv7-a/ia-64 instruction loads, and we don't need the write functions either. We only need the aligned read functions. I tried testing this for all four combinations of big/little endian host/target with a hello world program, and discovered that the big-endian host support is broken. The problem is with the GRegisterValue untion. You have typedef union GRegisterValue { int8_t s8; ... int64_t s64; } GRegister; On a little-endian host, the s8 member will match the low-byte of the s64 member, which is what we want. However, on a big-endian host, the s8 member will match the high-byte of the u64 member, and the simulator fails. I can fix this by using an anonymous struct for the big-endian case typedef union GRegisterValue { struct { int64_t :56; int8_t s8; }; ...l sint64_t s64; } GRegister; There are other ways to fix this, but this just seemed to me like the quickest and smallest patch that would make it work. There may also be other issues here, as I only tested an integer hello world program. Fixing the problem this way means that we require either an ISO C 2011 compiler, or a compiler that supports GCC extensions to ISO C 1990 or 1999. Otherwise, you may get an error for the anonymous structs. Or alternatively, it requires using a C++ compiler, as C++ added anonymous structs long before C did. I'm not sure how much of a problem this will be. If this is a serious problem, it could be fixed by giving names to the structs, adding the structs to the little endian side also with the field order switched, and then fixing all users to use the new names for the fields. That will be a bigger patch. With both changes, a hello world program works on all four combinations of big/little host/target. if you aren't happy with the cpustate.h change, it would be nice to get an approval for just the simulator.c change, as that is the part I care more about. We can worry about how to fix the big-endian host cpustate.h support later. Jim [-- Attachment #2: gdb-sim-aarch64.patch --] [-- Type: text/x-patch, Size: 1895 bytes --] 2016-06-29 Jim Wilson <jim.wilson@linaro.org> sim/aarch64/ * cpustate.h: Include config.h. (union GRegisterValue): Add WORDS_BIGENDIAN check. For big endian code use anonymous structs to align members. * simulator.c (aarch64_step): Use sim_core_read_buffer and endian_le2h_4 to read instruction from pc. diff --git a/sim/aarch64/cpustate.h b/sim/aarch64/cpustate.h index 07446a2..2754f7c 100644 --- a/sim/aarch64/cpustate.h +++ b/sim/aarch64/cpustate.h @@ -22,6 +22,7 @@ #ifndef _CPU_STATE_H #define _CPU_STATE_H +#include "config.h" #include <sys/types.h> #include <stdint.h> #include <inttypes.h> @@ -133,6 +134,7 @@ typedef enum VReg an explicit extend. */ typedef union GRegisterValue { +#if !WORDS_BIGENDIAN int8_t s8; int16_t s16; int32_t s32; @@ -141,6 +143,16 @@ typedef union GRegisterValue uint16_t u16; uint32_t u32; uint64_t u64; +#else + struct { int64_t :56; int8_t s8; }; + struct { int64_t :48; int16_t s16; }; + struct { int64_t :32; int32_t s32; }; + int64_t s64; + struct { uint64_t :56; uint8_t u8; }; + struct { uint64_t :48; uint16_t u16; }; + struct { uint64_t :32; uint32_t u32; }; + uint64_t u64; +#endif } GRegister; /* Float registers provide for storage of a single, double or quad diff --git a/sim/aarch64/simulator.c b/sim/aarch64/simulator.c index 88cb03d..8eb582a 100644 --- a/sim/aarch64/simulator.c +++ b/sim/aarch64/simulator.c @@ -14083,7 +14083,11 @@ aarch64_step (sim_cpu *cpu) return FALSE; aarch64_set_next_PC (cpu, pc + 4); - aarch64_get_instr (cpu) = aarch64_get_mem_u32 (cpu, pc); + + /* Code is always little-endian. */ + sim_core_read_buffer (CPU_STATE (cpu), cpu, read_map, + &aarch64_get_instr (cpu), pc, 4); + aarch64_get_instr (cpu) = endian_le2h_4 (aarch64_get_instr (cpu)); TRACE_INSN (cpu, " pc = %" PRIx64 " instr = %08x", pc, aarch64_get_instr (cpu)); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] aarch64 sim big-endian support 2016-06-30 1:33 ` Jim Wilson @ 2016-06-30 8:13 ` Nick Clifton 0 siblings, 0 replies; 6+ messages in thread From: Nick Clifton @ 2016-06-30 8:13 UTC (permalink / raw) To: Jim Wilson; +Cc: gdb-patches Hi Jim, > The problem is with the > GRegisterValue untion. You have > typedef union GRegisterValue > { > int8_t s8; > ... > int64_t s64; > } GRegister; > On a little-endian host, the s8 member will match the low-byte of the > s64 member, which is what we want. However, on a big-endian host, the > s8 member will match the high-byte of the u64 member, and the > simulator fails. Ah, yes. I had not considered this. > Fixing the problem this way means that we require either an ISO C 2011 > compiler, or a compiler that supports GCC extensions to ISO C 1990 or > 1999. I have no problems with this requirement. I have now checked in your patch (to save time). Thanks very much for persuing this matter. Cheers Nick ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] aarch64 sim big-endian support 2016-06-03 2:36 [PATCH] aarch64 sim big-endian support Jim Wilson 2016-06-10 16:24 ` Jim Wilson @ 2016-06-10 18:06 ` Mike Frysinger 1 sibling, 0 replies; 6+ messages in thread From: Mike Frysinger @ 2016-06-10 18:06 UTC (permalink / raw) To: Jim Wilson; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 703 bytes --] On 02 Jun 2016 19:36, Jim Wilson wrote: > aarch64_step (sim_cpu *cpu) > { > uint64_t pc = aarch64_get_PC (cpu); > + enum bfd_endian saved_target_byte_order; > > if (pc == TOP_LEVEL_RETURN_PC) > return FALSE; > > aarch64_set_next_PC (cpu, pc + 4); > + > + /* Code is always little-endian. */ > + saved_target_byte_order = current_target_byte_order; > + current_target_byte_order = BFD_ENDIAN_LITTLE; > aarch64_get_instr (cpu) = aarch64_get_mem_u32 (cpu, pc); > + current_target_byte_order = saved_target_byte_order; i don't think you should be messing with global state. the sim core has functions for reading raw opcodes if that's what you need. -mike [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-06-30 8:13 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-06-03 2:36 [PATCH] aarch64 sim big-endian support Jim Wilson 2016-06-10 16:24 ` Jim Wilson 2016-06-13 12:38 ` Nick Clifton 2016-06-30 1:33 ` Jim Wilson 2016-06-30 8:13 ` Nick Clifton 2016-06-10 18:06 ` Mike Frysinger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox