From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-x536.google.com (mail-ed1-x536.google.com [IPv6:2a00:1450:4864:20::536]) by sourceware.org (Postfix) with ESMTPS id BF6BB3858D35 for ; Tue, 14 May 2024 23:15:07 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BF6BB3858D35 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rivosinc.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org BF6BB3858D35 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2a00:1450:4864:20::536 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1715728510; cv=none; b=w4qpTIN4yQ6U7wc+YxgrjIedi3H9f41mfdwXcKa/NrOkehJhx1QE62soYJbfn1FAHQJwBh1y+/veP6fFQPFRT6GRZHoiy6Ez07KFceEVKKzOV5MNcpWvBEDfsDSpbNy7CYeRvs86BwRvmEyuD5MaTRt6ciia7gSPlOUQ5L+uiUg= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1715728510; c=relaxed/simple; bh=2TZVjrPZ2QtE9/rr+k/r+FNwubJLGXcel0hkrtqcanE=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=c5lyK8NS3N36eZJL0RhXc7it0GIz1vkLcv8xY+bsWLu0csdm39mPo6H/7BZanBvEXv0vUp3Wtpwjo8jeWdhit4eSEena/21Qca0QFf63IQ3gUC82t9UMxq9qBXo61PyCM/Rhrg6llba/VAmxbhe8SxuXq0rSrp8wBdOw8IpMx6c= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-ed1-x536.google.com with SMTP id 4fb4d7f45d1cf-572d83e3c7eso919094a12.3 for ; Tue, 14 May 2024 16:15:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20230601.gappssmtp.com; s=20230601; t=1715728506; x=1716333306; darn=sourceware.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=5uqss5Oo3JSTst+uZkpMlhDpa8Xn9/0hoVO/K7FNAfs=; b=amzxqpaybbRvctr1ASBixTIkqCUU1CButNSow/NBYg+C64/9XB1hXcu64Yyj4ec5OJ DFaqGdUeLkT1dvV+LK4yEWx5jAwhUm92SK2XmlAybRT2Iw0uE3H2cIrn5XLQmnWXlfdn jIp20rECGdZIFeN6AZDZO+QG77HfxkBLKaRzDxuko3YFvqg1n5FTjeI/NxxMs8x8vNpj M52wy+X0JGsehcmPUtsyuqtAY7LEmSIbLJTJbxyI7n+96BB9ddMGtzVs8/Ukp/89qCaf m1b1AhmOQ0ZSQPmga68z7fDBk2v5/LPL+qDWF4c4lg7qgJ5EEtIOFGIIxT5123HJE7iP y0SQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1715728506; x=1716333306; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=5uqss5Oo3JSTst+uZkpMlhDpa8Xn9/0hoVO/K7FNAfs=; b=MDJ4n8xrLkpCuHTrFvH0ZrzFG2ggsXezz7hh1nSmVRkYSJ9Qyw2CHAE7SM0WkTSFUT TQLKsVRPvrrHxZjTLO7ESfxBWGTf7mhxs2jQkip7a3cOU85R/xPAf41KRLUff4BMnfOM GkicZlDijx9GKpRsW1rKTaQlARlI/i0tFaBd9Ibs9z/7FbqqEznHsVQoyCfXrrplE+p9 aWT1ugBY38cypobc7HiHzqBSytqFpRVFNudcCXr6+WrOg+tzb6F086FG6gwb45LJqYaK rtrslR9+YK4d8SslWfWLGHHQstbfYVarTi53NMhlNh3I0yJoblt2oZbJNTeENdSx/0wN 1SRA== X-Forwarded-Encrypted: i=1; AJvYcCWPh2d1qiPW80STYCOX7LA1lRNuHATHlpd9e+K8CEpUGopXy1WjuCuS5V+/yubwii+EQ9UkTbonpB+eSE9JafKGUKhncWkjEA== X-Gm-Message-State: AOJu0YyEPZ1uf1/Ww7RLHrxPX+TMvCuhLEwnWY+7ZtqPllKMxK33OqSa o5GIFcqbTMfm/7iWeDBL9zyqv4KhBP2cNAY1GN4gpfPqO54Dd+p8HsHmRXj3sDoxhCNsuYijTGq 7jgcAKK9ykWrhiS1VlF2BSUbtJh+LlP90YWRY3urEreHwA9mP X-Google-Smtp-Source: AGHT+IE8dDMm2wNZBtoUbLfqWJDVFKk0rfnIAauGBrvxcxO1iq5xfN5qS6vTC1+WYarzAy3QvESfY5TIlkvdttYLTGs= X-Received: by 2002:a50:d4cb:0:b0:570:1ea8:cd1c with SMTP id 4fb4d7f45d1cf-5734d6b0e0amr10041164a12.35.1715728505775; Tue, 14 May 2024 16:15:05 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Nelson Chu Date: Wed, 15 May 2024 07:14:54 +0800 Message-ID: Subject: Re: [PATCH] RISC-V: Search for mapping symbols from the last one found To: Joseph Faulls Cc: Palmer Dabbelt , "binutils@sourceware.org" Content-Type: multipart/alternative; boundary="0000000000001e44e10618722c7f" X-Spam-Status: No, score=-9.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,GIT_PATCH_0,HTML_MESSAGE,HTTPS_HTTP_MISMATCH,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: --0000000000001e44e10618722c7f Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Thanks, committed ;) Nelson On Tue, May 14, 2024 at 5:10=E2=80=AFPM Joseph Faulls wrote: > Thanks, Nelson. Updated the patch: > > With previous behaviour, multiple mapping symbols within the same > function would result in all the mapping symbols being searched. > This could slow down disassembly dramatically. > > Multiple mapping symbols within a function can be a result of encoding > instructions as data, like sometimes seen in random instruction > generators. > > opcodes/ChangeLog: > > * riscv-dis.c (riscv_search_mapping_symbol): Use last mapping > symbol if it exists. > --- > opcodes/riscv-dis.c | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c > index 684098d386a..e6596c47423 100644 > --- a/opcodes/riscv-dis.c > +++ b/opcodes/riscv-dis.c > @@ -1076,11 +1076,9 @@ riscv_search_mapping_symbol (bfd_vma memaddr, > from_last_map_symbol =3D (last_map_symbol >=3D 0 > && info->stop_offset =3D=3D last_stop_offset); > > - /* Start scanning at the start of the function, or wherever > - we finished last time. */ > - n =3D info->symtab_pos + 1; > - if (from_last_map_symbol && n >=3D last_map_symbol) > - n =3D last_map_symbol; > + /* Start scanning from wherever we finished last time, or the start > + of the function. */ > + n =3D from_last_map_symbol ? last_map_symbol : info->symtab_pos + 1; > > /* Find the suitable mapping symbol to dump. */ > for (; n < info->symtab_size; n++) > @@ -1105,9 +1103,7 @@ riscv_search_mapping_symbol (bfd_vma memaddr, > can pick up a text mapping symbol of a preceeding section. */ > if (!found) > { > - n =3D info->symtab_pos; > - if (from_last_map_symbol && n >=3D last_map_symbol) > - n =3D last_map_symbol; > + n =3D from_last_map_symbol ? last_map_symbol : info->symtab_pos; > > for (; n >=3D 0; n--) > { > -- > 2.34.1 > > ------------------------------ > *From:* Nelson Chu > *Sent:* Tuesday, May 14, 2024 1:34 AM > *To:* Palmer Dabbelt > *Cc:* Joseph Faulls ; binutils@sourceware.org < > binutils@sourceware.org> > *Subject:* Re: [PATCH] RISC-V: Search for mapping symbols from the last > one found > > > There is a similar (n >=3D last_map_symbol) check when we need to backtra= ce > searching, https://github.com/bminor/binutils-gdb/blob/master/opcodes/ris= cv-dis.c#L1109 > [github.com] > , > should we also need to fix this? > > diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c > > index 684098d386a..e6596c47423 100644 > > --- a/opcodes/riscv-dis.c > > +++ b/opcodes/riscv-dis.c > > @@ -1076,11 +1076,9 @@ riscv_search_mapping_symbol (bfd_vma memaddr, > > from_last_map_symbol =3D (last_map_symbol >=3D 0 > && info->stop_offset =3D=3D last_stop_offset); > > - /* Start scanning at the start of the function, or wherever > > - we finished last time. */ > > - n =3D info->symtab_pos + 1; > > - if (from_last_map_symbol && n >=3D last_map_symbol) > > - n =3D last_map_symbol; > > + /* Start scanning from wherever we finished last time, or the start > > + of the function. */ > > + n =3D from_last_map_symbol ? last_map_symbol : info->symtab_pos + 1; > > > /* Find the suitable mapping symbol to dump. */ > for (; n < info->symtab_size; n++) > @@ -1105,9 +1103,7 @@ riscv_search_mapping_symbol (bfd_vma memaddr, > > can pick up a text mapping symbol of a preceeding section. */ > if (!found) > { > - n =3D info->symtab_pos; > > - if (from_last_map_symbol && n >=3D last_map_symbol) > > - n =3D last_map_symbol; > > + n =3D from_last_map_symbol ? last_map_symbol : info->symtab_pos; > > > for (; n >=3D 0; n--) > > On Mon, May 6, 2024 at 11:12=E2=80=AFPM Palmer Dabbelt wrote: > > On Fri, 03 May 2024 05:56:47 PDT (-0700), Joseph.Faulls@imgtec.com wrote: > > Ping. I know it's a small patch, but it sped up disassembly of my use > case from 5 minutes to 5 seconds. > > ________________________________ > > From: Joseph Faulls > > Sent: Thursday, April 18, 2024 11:48 AM > > To: binutils@sourceware.org > > Cc: nelson@rivosinc.com > > Subject: [PATCH] RISC-V: Search for mapping symbols from the last one > found > > > > With previous behaviour, multiple mapping symbols within the same > > function would result in all the mapping symbols being searched. > > This could slow down disassembly dramatically. > > > > Multiple mapping symbols within a function can be a result of encoding > > instructions as data, like sometimes seen in random instruction > > generators. > > > > opcodes/ChangeLog: > > > > * riscv-dis.c (riscv_search_mapping_symbol): Use last mapping > > symbol if it exists. > > > > Note for reviewers: > > I don't see why the previous check 'n >=3D last_map_symbol exists'. Why= do > we force starting from the start of the function if the last mapping symb= ol > was found after it? If I'm missing something, please let me know! > > Sorry for being slow here. Nelson and I were talking a bit right before > the weekend and it looks like this was just a bug, but it's very similar > to what the Arm code does. > > > I guess that is because we ported the code from aarch64 at the beginning, > and then kept their code to fix the stuff probably this one, https://gith= ub.com/bminor/binutils-gdb/commit/51457761649bab6868343b3da2258d73a62901f7 > [github.com] > . > But even applying Joseph's patch, the testcase from the above commit also > looks good, and I also passed the riscv-gnu-toolchain for the above chang= es > (two places use n >=3D last_map_symbol). So it seems no reason to keep t= he > (n >=3D last_map_symbol) checks for now. > > Nelson > --0000000000001e44e10618722c7f--