Re: [PATCH/MIPS] Add support Octeon's bbit instructions

Andrew Pinski
2012-08-19T22:19:39Z

[-- Attachment #1: Type: text/plain, Size: 1551 bytes --]

On Tue, Apr 24, 2012 at 12:27 PM, Maciej W. Rozycki
<macro@codesourcery.com> wrote:
> On Fri, 20 Apr 2012, Pinski, Andrew wrote:
>
>> Here is the updated patch with the style fixes and one extra change as I
>> noticed itype_op (inst) was being called a few times in mips32_next_pc,
>> I merged all of them into one variable.
>
> Thanks for doing this, I meant to do such a change as the next step.
> However I'd prefer functionally separate changes to be made as separate
> commits, so please split this change into two, first that eliminates the
> repetitive itype_op (inst) operations, and second that adds your new
> feature.
>
>> And renamed is_octeon_bit_op to
>> is_octeon_bbit_op since the instructions are named bbit and not bit.
>
> Thanks, that looks reasonable to me. Please also take into account my
> previous comment about function documentation that Joel has been kind
> enough to reiterate.
>
> OK with these changes.

These are the three patches I committed in the end. I also added a
testcase for testing bbit.

Thanks,
Andrew Pinski

Patch 1:
ChangeLog:
* mips-tdep.c (mips32_next_pc): Consolidate calls to itype_op.

Patch 2:
ChangeLog:
* mips-tdep.c (mips32_next_pc): Fix line spacing of the comment
before the function.

Patch 3:
ChangeLog:
* mips-tdep.c (is_octeon): New function.
(is_octeon_bbit_op): New function.
(mips32_next_pc): Handle Octeon's bbit instructions.
(mips32_instruction_has_delay_slot): Likewise.

testsuite/ChangeLog:
* gdb.arch/mips-octeon-bbit.c: New file.
* gdb.arch/mips-octeon-bbit.exp: New Test.

[-- Attachment #2: 0001-mips-tdep.c-mips32_next_pc-Consolidate-calls-to-ityp.patch --]
[-- Type: text/x-patch, Size: 2356 bytes --]

From a90e6c9cac622722a0dc046a19ab63e8fc23d9fe Mon Sep 17 00:00:00 2001
From: Andrew Pinski <apinski@cavium.com>
Date: Sun, 19 Aug 2012 11:24:48 -0700
Subject: [PATCH 1/3] * mips-tdep.c (mips32_next_pc): Consolidate calls to itype_op.

---
gdb/mips-tdep.c | 16 ++++++++--------
1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index a001424..07ae405 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -1475,14 +1475,14 @@ mips32_next_pc (struct frame_info *frame, CORE_ADDR pc)
unsigned long inst;
int op;
inst = mips_fetch_instruction (gdbarch, ISA_MIPS, pc, NULL);
+ op = itype_op (inst);
if ((inst & 0xe0000000) != 0) /* Not a special, jump or branch
instruction. */
{
- if (itype_op (inst) >> 2 == 5)
+ if (op >> 2 == 5)
/* BEQL, BNEL, BLEZL, BGTZL: bits 0101xx */
{
- op = (itype_op (inst) & 0x03);
- switch (op)
+ switch (op & 0x03)
{
case 0: /* BEQL */
goto equal_branch;
@@ -1496,18 +1496,18 @@ mips32_next_pc (struct frame_info *frame, CORE_ADDR pc)
pc += 4;
}
}
- else if (itype_op (inst) == 17 && itype_rs (inst) == 8)
+ else if (op == 17 && itype_rs (inst) == 8)
/* BC1F, BC1FL, BC1T, BC1TL: 010001 01000 */
pc = mips32_bc1_pc (gdbarch, frame, inst, pc + 4, 1);
- else if (itype_op (inst) == 17 && itype_rs (inst) == 9
+ else if (op == 17 && itype_rs (inst) == 9
&& (itype_rt (inst) & 2) == 0)
/* BC1ANY2F, BC1ANY2T: 010001 01001 xxx0x */
pc = mips32_bc1_pc (gdbarch, frame, inst, pc + 4, 2);
- else if (itype_op (inst) == 17 && itype_rs (inst) == 10
+ else if (op == 17 && itype_rs (inst) == 10
&& (itype_rt (inst) & 2) == 0)
/* BC1ANY4F, BC1ANY4T: 010001 01010 xxx0x */
pc = mips32_bc1_pc (gdbarch, frame, inst, pc + 4, 4);
- else if (itype_op (inst) == 29)
+ else if (op == 29)
/* JALX: 011101 */
/* The new PC will be alternate mode. */
{
@@ -1524,7 +1524,7 @@ mips32_next_pc (struct frame_info *frame, CORE_ADDR pc)
{ /* This gets way messy. */

/* Further subdivide into SPECIAL, REGIMM and other. */
- switch (op = itype_op (inst) & 0x07) /* Extract bits 28,27,26. */
+ switch (op & 0x07) /* Extract bits 28,27,26. */
{
case 0: /* SPECIAL */
op = rtype_funct (inst);
--
1.7.2.5

[-- Attachment #3: 0002-mips-tdep.c-mips32_next_pc-Fix-line-spacing-of-the-c.patch --]
[-- Type: text/x-patch, Size: 742 bytes --]

From e62b978677e9eab70b71a49f395aabcca2fae109 Mon Sep 17 00:00:00 2001
From: Andrew Pinski <apinski@cavium.com>
Date: Sun, 19 Aug 2012 11:57:39 -0700
Subject: [PATCH 2/3] * mips-tdep.c (mips32_next_pc): Fix line spacing of the comment
before the function.

---
gdb/mips-tdep.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 07ae405..751d7d6 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -1468,6 +1468,7 @@ mips32_bc1_pc (struct gdbarch *gdbarch, struct frame_info *frame,

/* Determine where to set a single step breakpoint while considering
branch prediction. */
+
static CORE_ADDR
mips32_next_pc (struct frame_info *frame, CORE_ADDR pc)
{
--
1.7.2.5

[-- Attachment #4: 0003-mips-tdep.c-is_octeon-New-function.patch --]
[-- Type: text/x-patch, Size: 7892 bytes --]

From 2a46d4e5ea7349ffe1deabee65379bfebf4ece46 Mon Sep 17 00:00:00 2001
From: Andrew Pinski <apinski@cavium.com>
Date: Sun, 19 Aug 2012 12:05:56 -0700
Subject: [PATCH 3/3] * mips-tdep.c (is_octeon): New function.
(is_octeon_bbit_op): New function.
(mips32_next_pc): Handle Octeon's bbit instructions.
(mips32_instruction_has_delay_slot): Likewise.

* gdb.arch/mips-octeon-bbit.c: New file.
* gdb.arch/mips-octeon-bbit.exp: New Test.
---
gdb/mips-tdep.c | 51 ++++++++++++-
gdb/testsuite/gdb.arch/mips-octeon-bbit.c | 49 ++++++++++++
gdb/testsuite/gdb.arch/mips-octeon-bbit.exp | 112 +++++++++++++++++++++++++++
3 files changed, 211 insertions(+), 1 deletions(-)
create mode 100644 gdb/testsuite/gdb.arch/mips-octeon-bbit.c
create mode 100644 gdb/testsuite/gdb.arch/mips-octeon-bbit.exp

diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c
index 751d7d6..56fa56c 100644
--- a/gdb/mips-tdep.c
+++ b/gdb/mips-tdep.c
@@ -1466,6 +1466,35 @@ mips32_bc1_pc (struct gdbarch *gdbarch, struct frame_info *frame,
return pc;
}

+/* Return nonzero if the gdbarch is an Octeon series. */
+
+static int
+is_octeon (struct gdbarch *gdbarch)
+{
+ const struct bfd_arch_info *info = gdbarch_bfd_arch_info (gdbarch);
+
+ return (info->mach == bfd_mach_mips_octeon
+ || info->mach == bfd_mach_mips_octeonp
+ || info->mach == bfd_mach_mips_octeon2);
+}
+
+/* Return true if the OP represents the Octeon's BBIT instruction. */
+
+static int
+is_octeon_bbit_op (int op, struct gdbarch *gdbarch)
+{
+ if (!is_octeon (gdbarch))
+ return 0;
+ /* BBIT0 is encoded as LWC2: 110 010. */
+ /* BBIT032 is encoded as LDC2: 110 110. */
+ /* BBIT1 is encoded as SWC2: 111 010. */
+ /* BBIT132 is encoded as SDC2: 111 110. */
+ if (op == 50 || op == 54 || op == 58 || op == 62)
+ return 1;
+ return 0;
+}
+
+
/* Determine where to set a single step breakpoint while considering
branch prediction. */

@@ -1518,6 +1547,25 @@ mips32_next_pc (struct frame_info *frame, CORE_ADDR pc)
/* Add 1 to indicate 16-bit mode -- invert ISA mode. */
pc = ((pc + 4) & ~(CORE_ADDR) 0x0fffffff) + reg + 1;
}
+ else if (is_octeon_bbit_op (op, gdbarch))
+ {
+ int bit, branch_if;
+
+ branch_if = op == 58 || op == 62;
+ bit = itype_rt (inst);
+
+ /* Take into account the *32 instructions. */
+ if (op == 54 || op == 62)
+ bit += 32;
+
+ if (((get_frame_register_signed (frame,
+ itype_rs (inst)) >> bit) & 1)
+ == branch_if)
+ pc += mips32_relative_offset (inst) + 4;
+ else
+ pc += 8; /* After the delay slot. */
+ }
+
else
pc += 4; /* Not a branch, next instruction is easy. */
}
@@ -6947,7 +6995,8 @@ mips32_instruction_has_delay_slot (struct gdbarch *gdbarch, CORE_ADDR addr)
{
rs = itype_rs (inst);
rt = itype_rt (inst);
- return (op >> 2 == 5 /* BEQL, BNEL, BLEZL, BGTZL: bits 0101xx */
+ return (is_octeon_bbit_op (op, gdbarch)
+ || op >> 2 == 5 /* BEQL, BNEL, BLEZL, BGTZL: bits 0101xx */
|| op == 29 /* JALX: bits 011101 */
|| (op == 17
&& (rs == 8
diff --git a/gdb/testsuite/gdb.arch/mips-octeon-bbit.c b/gdb/testsuite/gdb.arch/mips-octeon-bbit.c
new file mode 100644
index 0000000..e3df4a4
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/mips-octeon-bbit.c
@@ -0,0 +1,49 @@
+typedef unsigned long long uint64_t;
+void abort (void);
+
+#define BASE 0x1234567812345678ull
+
+#define DEF_BBIT_TAKEN(BRANCH_IF, BIT) \
+ int bbit_is_taken_##BRANCH_IF##_##BIT (volatile uint64_t *p) \
+ { \
+ int ret; \
+ asm (".set push \n\t" \
+ ".set noreorder \n\t" \
+ "bbit" #BRANCH_IF " %1, " #BIT ", 1f \n\t" \
+ "nop \n\t" \
+ "li %0, 0 \n\t" \
+ "b 2f \n\t" \
+ "nop \n\t" \
+ "1: \n\t" \
+ "li %0, 1 \n\t" \
+ "2: \n\t" \
+ ".set pop" \
+ : "=r"(ret) : "r"(*p)); \
+ return ret; \
+ } \
+ volatile uint64_t taken_##BRANCH_IF##_##BIT = \
+ BASE & (~(1ull << BIT)) | ((uint64_t) BRANCH_IF << BIT); \
+ volatile uint64_t not_taken_##BRANCH_IF##_##BIT = \
+ BASE & (~(1ull << BIT)) | (((uint64_t) !BRANCH_IF) << BIT);
+
+DEF_BBIT_TAKEN (0, 10);
+DEF_BBIT_TAKEN (0, 36);
+DEF_BBIT_TAKEN (1, 20);
+DEF_BBIT_TAKEN (1, 49);
+
+#define EXPECT(X) if (!(X)) abort ();
+
+main ()
+{
+ EXPECT (bbit_is_taken_0_10 (&taken_0_10));
+ EXPECT (!bbit_is_taken_0_10 (&not_taken_0_10));
+
+ EXPECT (bbit_is_taken_0_36 (&taken_0_36));
+ EXPECT (!bbit_is_taken_0_36 (&not_taken_0_36));
+
+ EXPECT (bbit_is_taken_1_20 (&taken_1_20));
+ EXPECT (!bbit_is_taken_1_20 (&not_taken_1_20));
+
+ EXPECT (bbit_is_taken_1_49 (&taken_1_49));
+ EXPECT (!bbit_is_taken_1_49 (&not_taken_1_49));
+}
diff --git a/gdb/testsuite/gdb.arch/mips-octeon-bbit.exp b/gdb/testsuite/gdb.arch/mips-octeon-bbit.exp
new file mode 100644
index 0000000..de6db0f
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/mips-octeon-bbit.exp
@@ -0,0 +1,112 @@
+# Copyright 2007 Cavium Networks, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+
+# Test single-step on bbit.
+
+if ![istarget "*octeon*"] {
+ return -1
+}
+
+proc current_insn {} {
+ global gdb_prompt
+
+ send_gdb "x/i \$pc\n"
+ gdb_expect {
+ -re ".*?:\\s+\(.*?\)\\s*$gdb_prompt $" {
+ set insn $expect_out(1,string)
+ return $insn
+ }
+ }
+ return ""
+}
+
+proc single_step {} {
+ global gdb_prompt
+
+ send_gdb "si\n"
+ gdb_expect {
+ -re "$gdb_prompt \$" {
+ return 1
+ }
+ }
+ return 0;
+}
+
+proc single_step_until { match } {
+ global timeout
+
+ set insn [current_insn]
+ set start [timestamp]
+ while { $insn != "" && [timestamp] - $start < 3*$timeout } {
+ if [regexp $match $insn] {
+ return 1
+ }
+ if {![single_step]} {
+ return 0
+ }
+ set insn [current_insn]
+ }
+ return 0;
+}
+
+proc test_bbit { name taken } {
+ if {![single_step_until "bbit"]} {
+ fail "$name single-step until bbit"
+ return
+ }
+ pass "$name single-step until bbit"
+ gdb_test "si" "" "$name single-step on bbit"
+ if [regexp "li\\s+\[sv\]0,$taken" [current_insn]] {
+ pass "$name check insn after bbit"
+ } else {
+ send_log "expected: li\\s+\[sv\]0,$taken found [current_insn]\n"
+ fail "$name check insn after bbit"
+ }
+}
+
+set testfile "mips-octeon-bbit"
+set srcfile ${testfile}.c
+set binfile ${objdir}/${subdir}/${testfile}
+
+if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable \
+ {debug nowarnings}] != "" } {
+ fail "compilation"
+ return
+}
+
+pass "compilation"
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+# Native needs run.
+runto_main
+
+set tests ""
+foreach n [list "0_10" "0_36" "1_20" "1_49"] {
+ lappend tests "bbit_is_taken_$n"
+}
+foreach func $tests {
+ gdb_test "break $func" "Breakpoint.*at.*" "set breakpoint on $func"
+}
+
+foreach func $tests {
+ gdb_test "continue" "Continuing.*Breakpoint.*$func.*" "hit $func first"
+ test_bbit "$func branch taken" 1
+ gdb_test "continue" "Continuing.*Breakpoint.*$func.*" "hit $func second"
+ test_bbit "$func branch not taken" 0
+}
--
1.7.2.5