Re: strub causing libgcc to fail to build on rl78-elf

Re: strub causing libgcc to fail to build on rl78-elf

[Alexandre Oliva]

[adding gcc@]

Hi, Jeff,

On Dec  6, 2023, Jeff Law <jeffreyalaw@gmail.com> wrote:

> libgcc is currently failing to build on rl78, looks like it might be
> strub related.

Thanks for letting me know.

>> /home/jlaw/test/gcc/libgcc/strub.c:149:1: error: unrecognizable insn:
>> 149 | }
>> | ^
>> (insn 30 64 33 4 (asm_operands/v ("") ("") 0 [
>> (mem/f:HI (reg:HI 8 r8) [1 *base_8+0 S2 A16])
>> ]
>> [
>> (asm_input:HI ("m") /home/jlaw/test/gcc/libgcc/strub.c:147)
>> ]
>> [] /home/jlaw/test/gcc/libgcc/strub.c:147) "/home/jlaw/test/gcc/libgcc/strub.c":147:3 -1
>> (expr_list:REG_DEAD (reg:HI 8 r8)
>> (nil)))
>> during RTL pass: cprop_hardreg

This looks like a latent bug in the port.

This was just a plain asm insn in strub.c:

  /* Make sure the stack overwrites are not optimized away.  */
  asm ("" : : "m" (end[0]));

whose constraint passes during reload, rl78_alloc_physical_registers
leaves it alone and clears virt_insns_ok, so when cprog_hardreg attempts
to extract constraints again, rl78_as_legitimate_address rejects r8 as
the address because virt insns are no longer ok.

I'm not at all familiar with this port, and I don't know what was
supposed to happen here, but ISTM that either physical registers should
be allocated for asms, or non-B0 regs should be accepted in asms.

I don't see a maintainer listed for rl78; is there anyone familiar
enough with it to confirm (or dispute ;-) my suspicions, and discuss
possibilities to get this fixed?

DJ, AFAICT was you who contributed the port.  Do you have any wisdom
about it to share?

TIA,

-- 
Alexandre Oliva, happy hacker                    https://FSFLA.org/blogs/lxo/
   Free Software Activist                           GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice but
very few check the facts.  Think Assange & Stallman.  The empires strike back

Re: strub causing libgcc to fail to build on rl78-elf

[DJ Delorie]

Alexandre Oliva <oliva@gnu.org> writes:
> This looks like a latent bug in the port.

I'm not surprised, that port was weird.

> This was just a plain asm insn in strub.c:
>
>   /* Make sure the stack overwrites are not optimized away.  */
>   asm ("" : : "m" (end[0]));
>
> whose constraint passes during reload, rl78_alloc_physical_registers
> leaves it alone and clears virt_insns_ok, so when cprog_hardreg attempts
> to extract constraints again, rl78_as_legitimate_address rejects r8 as
> the address because virt insns are no longer ok.

Some background: the "virtual" registers are memory-mapped registers
that use a common addressing scheme to access non-mapped registers.
When we convert from virtual to physical, we can map that reg to a
physical reg, or we replace the reg with a mem, but then usually have to
split up the insn.

For this insn, "converting" should have mapped or reloaded r8 to a valid
address register.  I doubt we have a way to have two patterns for user
asms like we do for, say, addhi3.

I suspect that something in the virt->phys logic just doesn't know how
to check for mem constraints in user asms.

> I'm not at all familiar with this port, and I don't know what was
> supposed to happen here, but ISTM that either physical registers should
> be allocated for asms, or non-B0 regs should be accepted in asms.

non-bank-zero registers aren't valid as real address registers, because
in gcc's reality they *are* mems.  The chip can bank switch them, but
gcc doesn't (the interrupt handlers, being asm, can do so, which is why
one bank is reserved for that).

Re: strub causing libgcc to fail to build on rl78-elf

[Jeff Law]

On 12/6/23 15:03, DJ Delorie wrote:
> Alexandre Oliva <oliva@gnu.org> writes:
>> This looks like a latent bug in the port.
> 
> I'm not surprised, that port was weird.
> 
>> This was just a plain asm insn in strub.c:
>>
>>    /* Make sure the stack overwrites are not optimized away.  */
>>    asm ("" : : "m" (end[0]));
>>
>> whose constraint passes during reload, rl78_alloc_physical_registers
>> leaves it alone and clears virt_insns_ok, so when cprog_hardreg attempts
>> to extract constraints again, rl78_as_legitimate_address rejects r8 as
>> the address because virt insns are no longer ok.
> 
> Some background: the "virtual" registers are memory-mapped registers
> that use a common addressing scheme to access non-mapped registers.
> When we convert from virtual to physical, we can map that reg to a
> physical reg, or we replace the reg with a mem, but then usually have to
> split up the insn.
> 
> For this insn, "converting" should have mapped or reloaded r8 to a valid
> address register.  I doubt we have a way to have two patterns for user
> asms like we do for, say, addhi3.
Given we don't know the semantics of what goes on inside the MEM I think 
rewriting would be extraordinarily difficult.  Ultimately all this feels 
like a limited reload pass implemented in the rl78 backend.  You'd have 
to look at all the operands, potentially spill one or more values, 
arrange for input/output reloads, deal with clobbers (particularly 
reloading the address given (clobber (mem (addr))).

> 
> I suspect that something in the virt->phys logic just doesn't know how
> to check for mem constraints in user asms.
I looked briefly, it appears the code just punts rewriting all user 
asms, but maybe I missed something.

I wouldn't lose any sleep if we had a way to simply disable strub for rl78.

Jeff

[PATCH] strub: disable on rl78

[Alexandre Oliva]

Hello, Jeff, DJ,

Thanks for the info.

On Dec  7, 2023, Jeff Law <jeffreyalaw@gmail.com> wrote:

> On 12/6/23 15:03, DJ Delorie wrote:
>> Alexandre Oliva <oliva@gnu.org> writes:
>>> This looks like a latent bug in the port.
>> I'm not surprised, that port was weird.
>> 
>>> This was just a plain asm insn in strub.c:
>>> 
>>> /* Make sure the stack overwrites are not optimized away.  */
>>> asm ("" : : "m" (end[0]));
>>> 
>>> whose constraint passes during reload, rl78_alloc_physical_registers
>>> leaves it alone and clears virt_insns_ok, so when cprog_hardreg attempts
>>> to extract constraints again, rl78_as_legitimate_address rejects r8 as
>>> the address because virt insns are no longer ok.
>> Some background: the "virtual" registers are memory-mapped registers
>> that use a common addressing scheme to access non-mapped registers.
>> When we convert from virtual to physical, we can map that reg to a
>> physical reg, or we replace the reg with a mem, but then usually have to
>> split up the insn.
>> For this insn, "converting" should have mapped or reloaded r8 to a
>> valid
>> address register.  I doubt we have a way to have two patterns for user
>> asms like we do for, say, addhi3.

*nod*, the virt-to-phys "reloader" should care of it it, but it doesn't
because it explicitly punts on most ASMs, and it sort-of implicitly
punts on inputs-only asms like the one in strub.c.

> Given we don't know the semantics of what goes on inside the MEM I
> think rewriting would be extraordinarily difficult.

ISTM that it would need to know what's inside, any more than it (or
reload) does.  It just needs to find (or make) some available phys
register and use that as the address instead of the virt register, like
it presumably does for other kinds of insns.

> I wouldn't lose any sleep if we had a way to simply disable strub for rl78.

Now that we have easy means to do that, it became irresistible ;-)

Tested building libgcc (all multilibs) with a cross rl78-elf.  Ok to install?


rl78 allocation of virtual registers to physical registers doesn't
operate on asm statements, and strub uses asm statements in the
runtime and in the generated code, to the point that the runtime
won't build.  Force strub disabled on that target.


for  gcc/ChangeLog

	* config/rl78/rl78.c (TARGET_HAVE_STRUB_SUPPORT_FOR): Disable.
---
 gcc/config/rl78/rl78.cc |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gcc/config/rl78/rl78.cc b/gcc/config/rl78/rl78.cc
index 5d8fddbd905a4..f3507280859b0 100644
--- a/gcc/config/rl78/rl78.cc
+++ b/gcc/config/rl78/rl78.cc
@@ -4972,6 +4972,11 @@ rl78_preferred_reload_class (rtx x ATTRIBUTE_UNUSED, reg_class_t rclass)
 }
 
 \f
+/* The strub runtime uses asms, and physical register allocation won't
+   deal with them, so disable it.  */
+#undef TARGET_HAVE_STRUB_SUPPORT_FOR
+#define TARGET_HAVE_STRUB_SUPPORT_FOR hook_bool_tree_false
+
 struct gcc_target targetm = TARGET_INITIALIZER;
 
 #include "gt-rl78.h"


-- 
Alexandre Oliva, happy hacker                    https://FSFLA.org/blogs/lxo/
   Free Software Activist                           GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice but
very few check the facts.  Think Assange & Stallman.  The empires strike back

Re: [PATCH] strub: disable on rl78

[Jeff Law]

On 12/8/23 19:18, Alexandre Oliva wrote:
> Hello, Jeff, DJ,
> 
> Thanks for the info.
> 
> On Dec  7, 2023, Jeff Law <jeffreyalaw@gmail.com> wrote:
> 
>> On 12/6/23 15:03, DJ Delorie wrote:
>>> Alexandre Oliva <oliva@gnu.org> writes:
>>>> This looks like a latent bug in the port.
>>> I'm not surprised, that port was weird.
>>>
>>>> This was just a plain asm insn in strub.c:
>>>>
>>>> /* Make sure the stack overwrites are not optimized away.  */
>>>> asm ("" : : "m" (end[0]));
>>>>
>>>> whose constraint passes during reload, rl78_alloc_physical_registers
>>>> leaves it alone and clears virt_insns_ok, so when cprog_hardreg attempts
>>>> to extract constraints again, rl78_as_legitimate_address rejects r8 as
>>>> the address because virt insns are no longer ok.
>>> Some background: the "virtual" registers are memory-mapped registers
>>> that use a common addressing scheme to access non-mapped registers.
>>> When we convert from virtual to physical, we can map that reg to a
>>> physical reg, or we replace the reg with a mem, but then usually have to
>>> split up the insn.
>>> For this insn, "converting" should have mapped or reloaded r8 to a
>>> valid
>>> address register.  I doubt we have a way to have two patterns for user
>>> asms like we do for, say, addhi3.
> 
> *nod*, the virt-to-phys "reloader" should care of it it, but it doesn't
> because it explicitly punts on most ASMs, and it sort-of implicitly
> punts on inputs-only asms like the one in strub.c.
> 
>> Given we don't know the semantics of what goes on inside the MEM I
>> think rewriting would be extraordinarily difficult.
> 
> ISTM that it would need to know what's inside, any more than it (or
> reload) does.  It just needs to find (or make) some available phys
> register and use that as the address instead of the virt register, like
> it presumably does for other kinds of insns.
> 
>> I wouldn't lose any sleep if we had a way to simply disable strub for rl78.
> 
> Now that we have easy means to do that, it became irresistible ;-)
> 
> Tested building libgcc (all multilibs) with a cross rl78-elf.  Ok to install?
> 
> 
> rl78 allocation of virtual registers to physical registers doesn't
> operate on asm statements, and strub uses asm statements in the
> runtime and in the generated code, to the point that the runtime
> won't build.  Force strub disabled on that target.
> 
> 
> for  gcc/ChangeLog
> 
> 	* config/rl78/rl78.c (TARGET_HAVE_STRUB_SUPPORT_FOR): Disable.
> ---
>   gcc/config/rl78/rl78.cc |    5 +++++
>   1 file changed, 5 insertions(+)
OK for the trunk.  THanks.

jeff

>