public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* objdump riscv: Stop disassembling addi rd, rs, 0 with a relocation as mv rd, rs?
@ 2023-02-07  2:40 Fangrui Song
  2023-02-07 14:39 ` Maciej W. Rozycki
  0 siblings, 1 reply; 6+ messages in thread
From: Fangrui Song @ 2023-02-07  2:40 UTC (permalink / raw)
  To: binutils

In llvm-project, https://reviews.llvm.org/D143345 brings up the topic
whether we should keep addi rd, rs, 0 when it is associated with a
relocation. If it does, the relocation may resolve to a non-zero and
`mv rd, rs` may look confusing.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: objdump riscv: Stop disassembling addi rd, rs, 0 with a relocation as mv rd, rs?
  2023-02-07  2:40 objdump riscv: Stop disassembling addi rd, rs, 0 with a relocation as mv rd, rs? Fangrui Song
@ 2023-02-07 14:39 ` Maciej W. Rozycki
  2023-02-07 15:14   ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2023-02-07 14:39 UTC (permalink / raw)
  To: Fangrui Song; +Cc: Jan Beulich, binutils

On Mon, 6 Feb 2023, Fangrui Song wrote:

> In llvm-project, https://reviews.llvm.org/D143345 brings up the topic
> whether we should keep addi rd, rs, 0 when it is associated with a
> relocation. If it does, the relocation may resolve to a non-zero and
> `mv rd, rs` may look confusing.

 I agree, I was quite amused when I came across it a while before, and I 
can imagine people may get confused.

 However it may be quite hard to implement given how the GNU disassembler 
has been structured; most easily probably by setting the `no_aliases' flag 
temporarily for any instruction seen with a relocation attached.  This 
could have undesired consequences elsewhere though and would most likely 
make Jan unhappy who wants to see aliases in disassembly rather than 
machine instruction mnemonics with the immediate forms.

 So instead we may need another flag for the opcode table, such as 
INSN_NORELOC, which would exclude the given entry for instructions with 
any relocation attached; I think this would be the cleanest approach, 
though maybe a little bit more involving implementation-wise.

 Overall I can see it as an unfortunate side effect of choosing `ADDI rd, 
rs, 0' rather than `ADD rd, rs, x0' (why?) for the canonical MV macro 
encoding.  Though of course it's the tools that have to adapt, not the 
other way round.

 FWIW,

  Maciej

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: objdump riscv: Stop disassembling addi rd, rs, 0 with a relocation as mv rd, rs?
  2023-02-07 14:39 ` Maciej W. Rozycki
@ 2023-02-07 15:14   ` Jan Beulich
  2023-02-07 15:44     ` Andreas Schwab
  2023-02-07 16:04     ` Maciej W. Rozycki
  0 siblings, 2 replies; 6+ messages in thread
From: Jan Beulich @ 2023-02-07 15:14 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: binutils, Fangrui Song

On 07.02.2023 15:39, Maciej W. Rozycki wrote:
> On Mon, 6 Feb 2023, Fangrui Song wrote:
>> In llvm-project, https://reviews.llvm.org/D143345 brings up the topic
>> whether we should keep addi rd, rs, 0 when it is associated with a
>> relocation. If it does, the relocation may resolve to a non-zero and
>> `mv rd, rs` may look confusing.
> 
>  I agree, I was quite amused when I came across it a while before, and I 
> can imagine people may get confused.

I agree as well.

>  However it may be quite hard to implement given how the GNU disassembler 
> has been structured; most easily probably by setting the `no_aliases' flag 
> temporarily for any instruction seen with a relocation attached.  This 
> could have undesired consequences elsewhere though and would most likely 
> make Jan unhappy who wants to see aliases in disassembly rather than 
> machine instruction mnemonics with the immediate forms.
> 
>  So instead we may need another flag for the opcode table, such as 
> INSN_NORELOC, which would exclude the given entry for instructions with 
> any relocation attached; I think this would be the cleanest approach, 
> though maybe a little bit more involving implementation-wise.

Could we get away with simply skipping aliases in general when there's a
relocation for an insn?

Jan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: objdump riscv: Stop disassembling addi rd, rs, 0 with a relocation as mv rd, rs?
  2023-02-07 15:14   ` Jan Beulich
@ 2023-02-07 15:44     ` Andreas Schwab
  2023-02-07 16:04     ` Maciej W. Rozycki
  1 sibling, 0 replies; 6+ messages in thread
From: Andreas Schwab @ 2023-02-07 15:44 UTC (permalink / raw)
  To: Jan Beulich via Binutils; +Cc: Maciej W. Rozycki, Jan Beulich, Fangrui Song

On Feb 07 2023, Jan Beulich via Binutils wrote:

> Could we get away with simply skipping aliases in general when there's a
> relocation for an insn?

There are aliases with relocatable operands that are not involved in the
alias selection. For example, all branch aliases are like that.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: objdump riscv: Stop disassembling addi rd, rs, 0 with a relocation as mv rd, rs?
  2023-02-07 15:14   ` Jan Beulich
  2023-02-07 15:44     ` Andreas Schwab
@ 2023-02-07 16:04     ` Maciej W. Rozycki
  2023-02-09  2:21       ` Nelson Chu
  1 sibling, 1 reply; 6+ messages in thread
From: Maciej W. Rozycki @ 2023-02-07 16:04 UTC (permalink / raw)
  To: Jan Beulich; +Cc: binutils, Fangrui Song

On Tue, 7 Feb 2023, Jan Beulich wrote:

> >  However it may be quite hard to implement given how the GNU disassembler 
> > has been structured; most easily probably by setting the `no_aliases' flag 
> > temporarily for any instruction seen with a relocation attached.  This 
> > could have undesired consequences elsewhere though and would most likely 
> > make Jan unhappy who wants to see aliases in disassembly rather than 
> > machine instruction mnemonics with the immediate forms.
> > 
> >  So instead we may need another flag for the opcode table, such as 
> > INSN_NORELOC, which would exclude the given entry for instructions with 
> > any relocation attached; I think this would be the cleanest approach, 
> > though maybe a little bit more involving implementation-wise.
> 
> Could we get away with simply skipping aliases in general when there's a
> relocation for an insn?

 I think it would be unreasonably inconsistent though.  Also I think we do 
want to keep the `*z' branch forms in disassembly.

  Maciej

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: objdump riscv: Stop disassembling addi rd, rs, 0 with a relocation as mv rd, rs?
  2023-02-07 16:04     ` Maciej W. Rozycki
@ 2023-02-09  2:21       ` Nelson Chu
  0 siblings, 0 replies; 6+ messages in thread
From: Nelson Chu @ 2023-02-09  2:21 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Jan Beulich, binutils, Fangrui Song, Andreas Schwab, Jim Wilson,
	Palmer Dabbelt, Andrew Waterman, Kito Cheng

Seems most of the experts agree with this good idea.  Besides, in the
other thread discussion, Palmer and I also like it,  so I created a
bugzilla to trace this,
https://sourceware.org/bugzilla/show_bug.cgi?id=30099

Thanks
Nelson

On Wed, Feb 8, 2023 at 12:05 AM Maciej W. Rozycki <macro@embecosm.com> wrote:
>
> On Tue, 7 Feb 2023, Jan Beulich wrote:
>
> > >  However it may be quite hard to implement given how the GNU disassembler
> > > has been structured; most easily probably by setting the `no_aliases' flag
> > > temporarily for any instruction seen with a relocation attached.  This
> > > could have undesired consequences elsewhere though and would most likely
> > > make Jan unhappy who wants to see aliases in disassembly rather than
> > > machine instruction mnemonics with the immediate forms.
> > >
> > >  So instead we may need another flag for the opcode table, such as
> > > INSN_NORELOC, which would exclude the given entry for instructions with
> > > any relocation attached; I think this would be the cleanest approach,
> > > though maybe a little bit more involving implementation-wise.
> >
> > Could we get away with simply skipping aliases in general when there's a
> > relocation for an insn?
>
>  I think it would be unreasonably inconsistent though.  Also I think we do
> want to keep the `*z' branch forms in disassembly.
>
>   Maciej

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-02-09  2:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-07  2:40 objdump riscv: Stop disassembling addi rd, rs, 0 with a relocation as mv rd, rs? Fangrui Song
2023-02-07 14:39 ` Maciej W. Rozycki
2023-02-07 15:14   ` Jan Beulich
2023-02-07 15:44     ` Andreas Schwab
2023-02-07 16:04     ` Maciej W. Rozycki
2023-02-09  2:21       ` Nelson Chu

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).