* [PATCH] fix for aarch64 sim tbnz bug @ 2016-12-02 4:49 Jim Wilson 2016-12-02 9:32 ` Mike Frysinger 2016-12-02 12:03 ` Nick Clifton 0 siblings, 2 replies; 11+ messages in thread From: Jim Wilson @ 2016-12-02 4:49 UTC (permalink / raw) To: gdb-patches; +Cc: Nick Clifton [-- Attachment #1: Type: text/plain, Size: 366 bytes --] 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 [-- Attachment #2: aarch64-sim-tbnz.patch --] [-- Type: text/x-patch, Size: 1367 bytes --] sim/aarch64 * simulator.c (tbnz, tbz): Cast 1 to uint64_t before shifting. (dexTestBranchImmediate): Shift high bit of pos by 5 not 4. diff --git a/sim/aarch64/simulator.c b/sim/aarch64/simulator.c index 4fa5dc1..34fd17d 100644 --- a/sim/aarch64/simulator.c +++ b/sim/aarch64/simulator.c @@ -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); ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix for aarch64 sim tbnz bug 2016-12-02 4:49 [PATCH] fix for aarch64 sim tbnz bug Jim Wilson @ 2016-12-02 9:32 ` Mike Frysinger 2016-12-05 3:45 ` Jim Wilson 2016-12-02 12:03 ` Nick Clifton 1 sibling, 1 reply; 11+ messages in thread From: Mike Frysinger @ 2016-12-02 9:32 UTC (permalink / raw) To: Jim Wilson; +Cc: gdb-patches, Nick Clifton 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix for aarch64 sim tbnz bug 2016-12-02 9:32 ` Mike Frysinger @ 2016-12-05 3:45 ` Jim Wilson 2016-12-12 18:38 ` Jim Wilson 2016-12-12 18:42 ` Mike Frysinger 0 siblings, 2 replies; 11+ messages in thread From: Jim Wilson @ 2016-12-05 3:45 UTC (permalink / raw) To: Mike Frysinger; +Cc: gdb-patches, Nick Clifton [-- Attachment #1: Type: text/plain, Size: 1127 bytes --] On Fri, Dec 2, 2016 at 1:31 AM, Mike Frysinger <vapier@gentoo.org> wrote: > can we please start getting tests added to sim too ? using gcc > indirectly to validate the sim is a bit un The GCC testsuite is the part that I care most about, but it does make sense for me to be adding simulator tests. There are currently no interesting aarch64 tests, so it isn't clear if there is any particular style I should be using. So I just chose a style that seemed OK to me. I have tests for the last two simulator patches I contributed. I verified that the tests work with current sources and fail without my two patches. I had to make a few fixes to the existing testsuite.inc file. I moved .Lpass and .Lfail out of the pass and fail macros, as otherwise I end up with duplicate definitions. I also changed fail to return non-zero, because it should. I find it confusing to manually run a testcase and see it print fail, and then return a zero exit code. Jim PS set_flags_for_add64 is broken, which causes adds and cmn to fail. I have a fix, and this will be my next aarch64 sim patch, but I haven't written a testcase yet. [-- Attachment #2: aarch64-sim-tests.patch --] [-- Type: text/x-patch, Size: 4294 bytes --] 2016-12-04 Jim Wilson <jim.wilson@linaro.org> sim/testsuite/sim/aarch64 * testutils.inc (pass): Move .Lpass outside macro. (fail): Move .Lfail outside macro. Return 1 instead of 0. * fstur.s: New. * tbnz.s: New. diff --git a/sim/testsuite/sim/aarch64/fstur.s b/sim/testsuite/sim/aarch64/fstur.s new file mode 100644 index 0000000..2206ae5 --- /dev/null +++ b/sim/testsuite/sim/aarch64/fstur.s @@ -0,0 +1,136 @@ +# mach: aarch64 + +# Check the FP store unscaled offset instructions: fsturs, fsturd, fsturq. +# Check the values -1, and XXX_MAX, which tests all bits. +# Check with offsets -256 and 255, which tests all bits. +# Also tests the FP load unscaled offset instructions: fldurs, fldurd, fldurq. + +.include "testutils.inc" + + .data +fm1: + .word 3212836864 +fmax: + .word 2139095039 +ftmp: + .word 0 + +dm1: + .word 0 + .word -1074790400 +dmax: + .word 4294967295 + .word 2146435071 +dtmp: + .word 0 + .word 0 + +ldm1: + .word 0 + .word 0 + .word 0 + .word -1073807360 +ldmax: + .word 4294967295 + .word 4294967295 + .word 4294967295 + .word 2147418111 +ldtmp: + .word 0 + .word 0 + .word 0 + .word 0 + + start + adrp x1, ftmp + add x1, x1, :lo12:ftmp + + adrp x0, fm1 + add x0, x0, :lo12:fm1 + sub x5, x0, #255 + sub x6, x1, #255 + movi d2, #0 + ldur s2, [x5, #255] + stur s2, [x6, #255] + ldr w3, [x0] + ldr w4, [x1] + cmp w3, w4 + bne .Lfailure + + adrp x0, fmax + add x0, x0, :lo12:fmax + add x5, x0, #256 + add x6, x1, #256 + movi d2, #0 + ldur s2, [x5, #-256] + stur s2, [x6, #-256] + ldr w3, [x0] + ldr w4, [x1] + cmp w3, w4 + bne .Lfailure + + adrp x1, dtmp + add x1, x1, :lo12:dtmp + + adrp x0, dm1 + add x0, x0, :lo12:dm1 + sub x5, x0, #255 + sub x6, x1, #255 + movi d2, #0 + ldur d2, [x5, #255] + stur d2, [x6, #255] + ldr x3, [x0] + ldr x4, [x1] + cmp x3, x4 + bne .Lfailure + + adrp x0, dmax + add x0, x0, :lo12:dmax + add x5, x0, #256 + add x6, x1, #256 + movi d2, #0 + ldur d2, [x5, #-256] + stur d2, [x6, #-256] + ldr x3, [x0] + ldr x4, [x1] + cmp x3, x4 + bne .Lfailure + + adrp x1, ldtmp + add x1, x1, :lo12:ldtmp + + adrp x0, ldm1 + add x0, x0, :lo12:ldm1 + sub x5, x0, #255 + sub x6, x1, #255 + movi v2.2d, #0 + ldur q2, [x5, #255] + stur q2, [x6, #255] + ldr x3, [x0] + ldr x4, [x1] + cmp x3, x4 + bne .Lfailure + ldr x3, [x0, 8] + ldr x4, [x1, 8] + cmp x3, x4 + bne .Lfailure + + adrp x0, ldmax + add x0, x0, :lo12:ldmax + add x5, x0, #256 + add x6, x1, #256 + movi v2.2d, #0 + ldur q2, [x5, #-256] + stur q2, [x6, #-256] + ldr x3, [x0] + ldr x4, [x1] + cmp x3, x4 + bne .Lfailure + ldr x3, [x0, 8] + ldr x4, [x1, 8] + cmp x3, x4 + bne .Lfailure + + pass +.Lfailure: + fail diff --git a/sim/testsuite/sim/aarch64/tbnz.s b/sim/testsuite/sim/aarch64/tbnz.s new file mode 100644 index 0000000..2416101 --- /dev/null +++ b/sim/testsuite/sim/aarch64/tbnz.s @@ -0,0 +1,55 @@ +# mach: aarch64 + +# Check the test-bit-and-branch instructions: tbnz, and tbz. +# We check the edge condition bit positions: 0, 1<<31, 1<<32, 1<<63. + +.include "testutils.inc" + + start + mov x0, #1 + tbnz x0, #0, .L1 + fail +.L1: + tbz x0, #0, .Lfailure + mov x0, #0xFFFFFFFFFFFFFFFE + tbnz x0, #0, .Lfailure + tbz x0, #0, .L2 + fail +.L2: + + mov x0, #0x80000000 + tbnz x0, #31, .L3 + fail +.L3: + tbz x0, #31, .Lfailure + mov x0, #0xFFFFFFFF7FFFFFFF + tbnz x0, #31, .Lfailure + tbz x0, #31, .L4 + fail +.L4: + + mov x0, #0x100000000 + tbnz x0, #32, .L5 + fail +.L5: + tbz x0, #32, .Lfailure + mov x0, #0xFFFFFFFEFFFFFFFF + tbnz x0, #32, .Lfailure + tbz x0, #32, .L6 + fail +.L6: + + mov x0, #0x8000000000000000 + tbnz x0, #63, .L7 + fail +.L7: + tbz x0, #63, .Lfailure + mov x0, #0x7FFFFFFFFFFFFFFF + tbnz x0, #63, .Lfailure + tbz x0, #63, .L8 + fail +.L8: + + pass +.Lfailure: + fail diff --git a/sim/testsuite/sim/aarch64/testutils.inc b/sim/testsuite/sim/aarch64/testutils.inc index c8897aa..99057af 100644 --- a/sim/testsuite/sim/aarch64/testutils.inc +++ b/sim/testsuite/sim/aarch64/testutils.inc @@ -43,11 +43,11 @@ swiwrite 5 exit 0 + .endm .data .Lpass: .asciz "pass\n" - .endm # MACRO: fail # Write 'fail' to stdout and quit @@ -56,12 +56,12 @@ adrp x1, .Lfail add x1, x1, :lo12:.Lfail swiwrite 5 - exit 0 + exit 1 + .endm .data .Lfail: .asciz "fail\n" - .endm # MACRO: start # All assembler tests should start with a call to "start" ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix for aarch64 sim tbnz bug 2016-12-05 3:45 ` Jim Wilson @ 2016-12-12 18:38 ` Jim Wilson 2016-12-12 18:42 ` Mike Frysinger 1 sibling, 0 replies; 11+ messages in thread From: Jim Wilson @ 2016-12-12 18:38 UTC (permalink / raw) To: Nick Clifton; +Cc: gdb-patches ping https://sourceware.org/ml/gdb-patches/2016-12/msg00128.html On Sun, Dec 4, 2016 at 7:43 PM, Jim Wilson <jim.wilson@linaro.org> wrote: > On Fri, Dec 2, 2016 at 1:31 AM, Mike Frysinger <vapier@gentoo.org> wrote: >> can we please start getting tests added to sim too ? using gcc >> indirectly to validate the sim is a bit un > > The GCC testsuite is the part that I care most about, but it does make > sense for me to be adding simulator tests. There are currently no > interesting aarch64 tests, so it isn't clear if there is any > particular style I should be using. So I just chose a style that > seemed OK to me. I have tests for the last two simulator patches I > contributed. I verified that the tests work with current sources and > fail without my two patches. > > I had to make a few fixes to the existing testsuite.inc file. I moved > .Lpass and .Lfail out of the pass and fail macros, as otherwise I end > up with duplicate definitions. I also changed fail to return > non-zero, because it should. I find it confusing to manually run a > testcase and see it print fail, and then return a zero exit code. > > Jim > > PS set_flags_for_add64 is broken, which causes adds and cmn to fail. > I have a fix, and this will be my next aarch64 sim patch, but I > haven't written a testcase yet. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix for aarch64 sim tbnz bug 2016-12-05 3:45 ` Jim Wilson 2016-12-12 18:38 ` Jim Wilson @ 2016-12-12 18:42 ` Mike Frysinger 2016-12-13 0:33 ` Jim Wilson 1 sibling, 1 reply; 11+ messages in thread From: Mike Frysinger @ 2016-12-12 18:42 UTC (permalink / raw) To: Jim Wilson; +Cc: gdb-patches, Nick Clifton [-- Attachment #1: Type: text/plain, Size: 760 bytes --] On 04 Dec 2016 19:43, Jim Wilson wrote: > --- a/sim/testsuite/sim/aarch64/testutils.inc > +++ b/sim/testsuite/sim/aarch64/testutils.inc > @@ -43,11 +43,11 @@ > > swiwrite 5 > exit 0 > + .endm > > .data > .Lpass: > .asciz "pass\n" > - .endm > > # MACRO: fail > # Write 'fail' to stdout and quit > @@ -56,12 +56,12 @@ > adrp x1, .Lfail > add x1, x1, :lo12:.Lfail > swiwrite 5 > - exit 0 > + exit 1 > + .endm > > .data > .Lfail: > .asciz "fail\n" > - .endm > > # MACRO: start > # All assembler tests should start with a call to "start" i think you want to stuff those pass/fail strings into the start macro instead of just having it get inserted where the include happened to show up. -mike [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix for aarch64 sim tbnz bug 2016-12-12 18:42 ` Mike Frysinger @ 2016-12-13 0:33 ` Jim Wilson 2016-12-13 6:38 ` Mike Frysinger 2016-12-13 10:45 ` Nick Clifton 0 siblings, 2 replies; 11+ messages in thread From: Jim Wilson @ 2016-12-13 0:33 UTC (permalink / raw) To: Jim Wilson, gdb-patches, Nick Clifton [-- Attachment #1: Type: text/plain, Size: 362 bytes --] On Mon, Dec 12, 2016 at 10:42 AM, Mike Frysinger <vapier@gentoo.org> wrote: > i think you want to stuff those pass/fail strings into the > start macro instead of just having it get inserted where the > include happened to show up. That makes sense. Here is a new version of the patch with that change. Retested with a sim make check, and all tests pass. Jim [-- Attachment #2: aarch64-sim-tests.patch --] [-- Type: text/x-patch, Size: 4387 bytes --] 2016-12-12 Jim Wilson <jim.wilson@linaro.org> sim/testsuite/sim/aarch64 * testutils.inc (pass): Move .Lpass to start. (fail): Move .Lfail to start. Return 1 instead of 0. (start): Moved .Lpass and .Lfail to here. * fstur.s: New. * tbnz.s: New. diff --git a/sim/testsuite/sim/aarch64/fstur.s b/sim/testsuite/sim/aarch64/fstur.s new file mode 100644 index 0000000..2206ae5 --- /dev/null +++ b/sim/testsuite/sim/aarch64/fstur.s @@ -0,0 +1,136 @@ +# mach: aarch64 + +# Check the FP store unscaled offset instructions: fsturs, fsturd, fsturq. +# Check the values -1, and XXX_MAX, which tests all bits. +# Check with offsets -256 and 255, which tests all bits. +# Also tests the FP load unscaled offset instructions: fldurs, fldurd, fldurq. + +.include "testutils.inc" + + .data +fm1: + .word 3212836864 +fmax: + .word 2139095039 +ftmp: + .word 0 + +dm1: + .word 0 + .word -1074790400 +dmax: + .word 4294967295 + .word 2146435071 +dtmp: + .word 0 + .word 0 + +ldm1: + .word 0 + .word 0 + .word 0 + .word -1073807360 +ldmax: + .word 4294967295 + .word 4294967295 + .word 4294967295 + .word 2147418111 +ldtmp: + .word 0 + .word 0 + .word 0 + .word 0 + + start + adrp x1, ftmp + add x1, x1, :lo12:ftmp + + adrp x0, fm1 + add x0, x0, :lo12:fm1 + sub x5, x0, #255 + sub x6, x1, #255 + movi d2, #0 + ldur s2, [x5, #255] + stur s2, [x6, #255] + ldr w3, [x0] + ldr w4, [x1] + cmp w3, w4 + bne .Lfailure + + adrp x0, fmax + add x0, x0, :lo12:fmax + add x5, x0, #256 + add x6, x1, #256 + movi d2, #0 + ldur s2, [x5, #-256] + stur s2, [x6, #-256] + ldr w3, [x0] + ldr w4, [x1] + cmp w3, w4 + bne .Lfailure + + adrp x1, dtmp + add x1, x1, :lo12:dtmp + + adrp x0, dm1 + add x0, x0, :lo12:dm1 + sub x5, x0, #255 + sub x6, x1, #255 + movi d2, #0 + ldur d2, [x5, #255] + stur d2, [x6, #255] + ldr x3, [x0] + ldr x4, [x1] + cmp x3, x4 + bne .Lfailure + + adrp x0, dmax + add x0, x0, :lo12:dmax + add x5, x0, #256 + add x6, x1, #256 + movi d2, #0 + ldur d2, [x5, #-256] + stur d2, [x6, #-256] + ldr x3, [x0] + ldr x4, [x1] + cmp x3, x4 + bne .Lfailure + + adrp x1, ldtmp + add x1, x1, :lo12:ldtmp + + adrp x0, ldm1 + add x0, x0, :lo12:ldm1 + sub x5, x0, #255 + sub x6, x1, #255 + movi v2.2d, #0 + ldur q2, [x5, #255] + stur q2, [x6, #255] + ldr x3, [x0] + ldr x4, [x1] + cmp x3, x4 + bne .Lfailure + ldr x3, [x0, 8] + ldr x4, [x1, 8] + cmp x3, x4 + bne .Lfailure + + adrp x0, ldmax + add x0, x0, :lo12:ldmax + add x5, x0, #256 + add x6, x1, #256 + movi v2.2d, #0 + ldur q2, [x5, #-256] + stur q2, [x6, #-256] + ldr x3, [x0] + ldr x4, [x1] + cmp x3, x4 + bne .Lfailure + ldr x3, [x0, 8] + ldr x4, [x1, 8] + cmp x3, x4 + bne .Lfailure + + pass +.Lfailure: + fail diff --git a/sim/testsuite/sim/aarch64/tbnz.s b/sim/testsuite/sim/aarch64/tbnz.s new file mode 100644 index 0000000..2416101 --- /dev/null +++ b/sim/testsuite/sim/aarch64/tbnz.s @@ -0,0 +1,55 @@ +# mach: aarch64 + +# Check the test-bit-and-branch instructions: tbnz, and tbz. +# We check the edge condition bit positions: 0, 1<<31, 1<<32, 1<<63. + +.include "testutils.inc" + + start + mov x0, #1 + tbnz x0, #0, .L1 + fail +.L1: + tbz x0, #0, .Lfailure + mov x0, #0xFFFFFFFFFFFFFFFE + tbnz x0, #0, .Lfailure + tbz x0, #0, .L2 + fail +.L2: + + mov x0, #0x80000000 + tbnz x0, #31, .L3 + fail +.L3: + tbz x0, #31, .Lfailure + mov x0, #0xFFFFFFFF7FFFFFFF + tbnz x0, #31, .Lfailure + tbz x0, #31, .L4 + fail +.L4: + + mov x0, #0x100000000 + tbnz x0, #32, .L5 + fail +.L5: + tbz x0, #32, .Lfailure + mov x0, #0xFFFFFFFEFFFFFFFF + tbnz x0, #32, .Lfailure + tbz x0, #32, .L6 + fail +.L6: + + mov x0, #0x8000000000000000 + tbnz x0, #63, .L7 + fail +.L7: + tbz x0, #63, .Lfailure + mov x0, #0x7FFFFFFFFFFFFFFF + tbnz x0, #63, .Lfailure + tbz x0, #63, .L8 + fail +.L8: + + pass +.Lfailure: + fail diff --git a/sim/testsuite/sim/aarch64/testutils.inc b/sim/testsuite/sim/aarch64/testutils.inc index c8897aa..1fc9bc8 100644 --- a/sim/testsuite/sim/aarch64/testutils.inc +++ b/sim/testsuite/sim/aarch64/testutils.inc @@ -43,10 +43,6 @@ swiwrite 5 exit 0 - - .data -.Lpass: - .asciz "pass\n" .endm # MACRO: fail @@ -56,16 +52,18 @@ adrp x1, .Lfail add x1, x1, :lo12:.Lfail swiwrite 5 - exit 0 - - .data -.Lfail: - .asciz "fail\n" + exit 1 .endm # MACRO: start # All assembler tests should start with a call to "start" .macro start + .data +.Lpass: + .asciz "pass\n" +.Lfail: + .asciz "fail\n" + .text .global _start _start: ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix for aarch64 sim tbnz bug 2016-12-13 0:33 ` Jim Wilson @ 2016-12-13 6:38 ` Mike Frysinger 2016-12-13 10:45 ` Nick Clifton 1 sibling, 0 replies; 11+ messages in thread From: Mike Frysinger @ 2016-12-13 6:38 UTC (permalink / raw) To: Jim Wilson; +Cc: gdb-patches, Nick Clifton [-- Attachment #1: Type: text/plain, Size: 525 bytes --] On 12 Dec 2016 16:32, Jim Wilson wrote: > On Mon, Dec 12, 2016 at 10:42 AM, Mike Frysinger wrote: > > i think you want to stuff those pass/fail strings into the > > start macro instead of just having it get inserted where the > > include happened to show up. > > That makes sense. Here is a new version of the patch with that > change. Retested with a sim make check, and all tests pass. at a high level, looks good. i haven't gotten into aarch64 (yet?), so can't really comment on the test specifics. -mike [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix for aarch64 sim tbnz bug 2016-12-13 0:33 ` Jim Wilson 2016-12-13 6:38 ` Mike Frysinger @ 2016-12-13 10:45 ` Nick Clifton 1 sibling, 0 replies; 11+ messages in thread From: Nick Clifton @ 2016-12-13 10:45 UTC (permalink / raw) To: Jim Wilson, gdb-patches Hi Jim, > That makes sense. Here is a new version of the patch with that > change. Retested with a sim make check, and all tests pass. Excellent - thanks very much for doing this. Patch approved. Cheers Nick ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix for aarch64 sim tbnz bug 2016-12-02 4:49 [PATCH] fix for aarch64 sim tbnz bug Jim Wilson 2016-12-02 9:32 ` Mike Frysinger @ 2016-12-02 12:03 ` Nick Clifton 2016-12-02 13:34 ` Andreas Schwab 2016-12-02 15:59 ` Jim Wilson 1 sibling, 2 replies; 11+ messages in thread From: Nick Clifton @ 2016-12-02 12:03 UTC (permalink / raw) To: Jim Wilson, gdb-patches 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix for aarch64 sim tbnz bug 2016-12-02 12:03 ` Nick Clifton @ 2016-12-02 13:34 ` Andreas Schwab 2016-12-02 15:59 ` Jim Wilson 1 sibling, 0 replies; 11+ messages in thread From: Andreas Schwab @ 2016-12-02 13:34 UTC (permalink / raw) To: Nick Clifton; +Cc: Jim Wilson, gdb-patches 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. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] fix for aarch64 sim tbnz bug 2016-12-02 12:03 ` Nick Clifton 2016-12-02 13:34 ` Andreas Schwab @ 2016-12-02 15:59 ` Jim Wilson 1 sibling, 0 replies; 11+ messages in thread From: Jim Wilson @ 2016-12-02 15:59 UTC (permalink / raw) To: Nick Clifton; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-12-13 10:45 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-12-02 4:49 [PATCH] fix for aarch64 sim tbnz bug Jim Wilson 2016-12-02 9:32 ` Mike Frysinger 2016-12-05 3:45 ` Jim Wilson 2016-12-12 18:38 ` Jim Wilson 2016-12-12 18:42 ` Mike Frysinger 2016-12-13 0:33 ` Jim Wilson 2016-12-13 6:38 ` Mike Frysinger 2016-12-13 10:45 ` Nick Clifton 2016-12-02 12:03 ` Nick Clifton 2016-12-02 13:34 ` Andreas Schwab 2016-12-02 15:59 ` Jim Wilson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).