mbox series

[0/2] RISC-V: a little more macro insn handling adjusting

Message ID 8255d3af-23e4-054d-be6c-28fae6a76ea0@suse.com
Headers
Series RISC-V: a little more macro insn handling adjusting |

Message

Jan Beulich Nov. 3, 2023, 12:55 p.m. UTC 
  1: disallow x0 with certain macro-insns
2: reduce redundancy in sign/zero extension macro insn handling

See also the remark in patch 1 towards possible further tidying.

Jan

Comments

Jan Beulich Nov. 17, 2023, 10:18 a.m. UTC | #1 
On 03.11.2023 13:55, Jan Beulich wrote:
> 1: disallow x0 with certain macro-insns
> 2: reduce redundancy in sign/zero extension macro insn handling

Not hearing anything back kind of suggests no objections. I'll commit
these two patches at the end of next week unless I hear back earlier.

Jan

> See also the remark in patch 1 towards possible further tidying.
> 
> Jan
Palmer Dabbelt Nov. 22, 2023, 12:26 a.m. UTC | #2 
On Fri, 17 Nov 2023 02:18:12 PST (-0800), jbeulich@suse.com wrote:
> On 03.11.2023 13:55, Jan Beulich wrote:
>> 1: disallow x0 with certain macro-insns
>> 2: reduce redundancy in sign/zero extension macro insn handling
>
> Not hearing anything back kind of suggests no objections. I'll commit
> these two patches at the end of next week unless I hear back earlier.

Sorry for being slow here.  I don't really have a strong feeling either 
way: having x0 in these macros results in legal instructions, even if 
they're probably semanticly useless (I suppose one can map address 0, 
but that's probably a bad idea).  So it's possible we break users here, 
but maybe they were all just doing something wrong to begin with?

So

Acked-by: Palmer Dabbelt <palmer@rivosinc.com>

if you want to take them, I'm fine either way.

We should probably put something in NEWS, though -- maybe nobody really 
reads it, but at least we can point to it if something breaks ;)

>
> Jan
>
>> See also the remark in patch 1 towards possible further tidying.
>>
>> Jan
Andrew Waterman Nov. 22, 2023, 12:39 a.m. UTC | #3 
On Tue, Nov 21, 2023 at 4:26 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Fri, 17 Nov 2023 02:18:12 PST (-0800), jbeulich@suse.com wrote:
> > On 03.11.2023 13:55, Jan Beulich wrote:
> >> 1: disallow x0 with certain macro-insns
> >> 2: reduce redundancy in sign/zero extension macro insn handling
> >
> > Not hearing anything back kind of suggests no objections. I'll commit
> > these two patches at the end of next week unless I hear back earlier.
>
> Sorry for being slow here.  I don't really have a strong feeling either
> way: having x0 in these macros results in legal instructions, even if
> they're probably semanticly useless (I suppose one can map address 0,
> but that's probably a bad idea).

There are legitimate use.cases for using x0 as a base register, e.g.
the debug ROM code which takes advantage of the fact that the debug
module registers live at the low addresses:
https://github.com/riscv-software-src/riscv-isa-sim/blob/4841ad0238f0b71ca86fb28974765495cc0c34a9/debug_rom/debug_rom.S#L35

That use case isn't broken by this change, since it uses a bona fide
instruction, not a macro.  I only mention it to point out that we
wouldn't want to ban use of x0 as a base address in non-macro
contexts.

>  So it's possible we break users here,
> but maybe they were all just doing something wrong to begin with?
>
> So
>
> Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
>
> if you want to take them, I'm fine either way.
>
> We should probably put something in NEWS, though -- maybe nobody really
> reads it, but at least we can point to it if something breaks ;)
>
> >
> > Jan
> >
> >> See also the remark in patch 1 towards possible further tidying.
> >>
> >> Jan
Jan Beulich Nov. 22, 2023, 7:59 a.m. UTC | #4 
On 22.11.2023 01:39, Andrew Waterman wrote:
> On Tue, Nov 21, 2023 at 4:26 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>
>> On Fri, 17 Nov 2023 02:18:12 PST (-0800), jbeulich@suse.com wrote:
>>> On 03.11.2023 13:55, Jan Beulich wrote:
>>>> 1: disallow x0 with certain macro-insns
>>>> 2: reduce redundancy in sign/zero extension macro insn handling
>>>
>>> Not hearing anything back kind of suggests no objections. I'll commit
>>> these two patches at the end of next week unless I hear back earlier.
>>
>> Sorry for being slow here.  I don't really have a strong feeling either
>> way: having x0 in these macros results in legal instructions, even if
>> they're probably semanticly useless (I suppose one can map address 0,
>> but that's probably a bad idea).
> 
> There are legitimate use.cases for using x0 as a base register, e.g.
> the debug ROM code which takes advantage of the fact that the debug
> module registers live at the low addresses:
> https://github.com/riscv-software-src/riscv-isa-sim/blob/4841ad0238f0b71ca86fb28974765495cc0c34a9/debug_rom/debug_rom.S#L35
> 
> That use case isn't broken by this change, since it uses a bona fide
> instruction, not a macro.  I only mention it to point out that we
> wouldn't want to ban use of x0 as a base address in non-macro
> contexts.

Right, we had that discussion a year or two ago. But as you imply, I
didn't see that extending to these macros insns.

Jan