On 9/28/23 15:43, Vineet Gupta wrote:
RISC-V suffers from extraneous sign extensions, despite/given the ABI
guarantee that 32-bit quantities are sign-extended into 64-bit registers,
meaning incoming SI function args need not be explicitly sign extended
(so do SI return values as most ALU insns implicitly sign-extend too.)

Existing REE doesn't seem to handle this well and there are various ideas
floating around to smarten REE about it.

RISC-V also seems to correctly implement middle-end hook PROMOTE_MODE
etc.

Another approach would be to prevent EXPAND from generating the
sign_extend in the first place which this patch tries to do.

The hunk being removed was introduced way back in 1994 as
    5069803972 ("expand_expr, case CONVERT_EXPR .. clear the promotion flag")

This survived full testsuite run for RISC-V rv64gc with surprisingly no
fallouts: test results before/after are exactly same.

|                               | # of unexpected case / # of unique unexpected 
case
|                               |          gcc |          g++ |     gfortran |
| rv64imafdc_zba_zbb_zbs_zicond/|  264 /    87 |    5 /     2 |   72 /    12 |
|    lp64d/medlow

Granted for something so old to have survived, there must be a valid
reason. Unfortunately the original change didn't have additional
commentary or a test case. That is not to say it can't/won't possibly
break things on other arches/ABIs, hence the RFC for someone to scream
that this is just bonkers, don't do this :-)

I've explicitly CC'ed Jakub and Roger who have last touched subreg
promoted notes in expr.cc for insight and/or screaming ;-)

Thanks to Robin for narrowing this down in an amazing debugging session
@ GNU Cauldron.
So I scoured my old gcc2 archives to see if there was anything that might hint as to why this was changed. Sadly (but not unexpectedly), nothing. The relevant ChangeLog entry is;


Fri Jul  8 11:46:50 1994  Richard Kenner  (ken...@vlsi1.ultra.nyu.edu)

        * varasm.c (record_constant_rtx, force_const_mem): Ensure everything
        is in saveable_obstack, not current_obstack.

        * combine.c (force_to_mode): OP_MODE must be MODE if MODE and
        mode of X are of different classes.
        (nonzero_bits, num_sign_bit_copies): Say nothing known for
        floating-point modes.

        * function.c (instantiate_virtual_regs_1, case SET):
        If DEST is virtual_stack_vars_rtx, replace with hardware
        frame pointer.

        * expr.c (expand_expr, case CONVERT_EXPR): If changing signedness
        and we have a promoted SUBREG, clear the promotion flag.

        * c-decl.c (finish_decl): Put RTL and other stuff in
        permanent_obstack if DECL is.

        * combine.c (gen_unary): Add new arg, OP0_MODE.
        All callers changed.

So standard practice back then was to re-use the header and have a blank line between conceptual changes if the same author made a series of changes. So it's reasonable to assume the expr.c change was considered independent of the other changes.

At that particular time I think Kenner was mostly focused on the alpha and ppc ports, but I think he was also still poking around with romp and a29k. I think romp is an unlikely target for this because it didn't promote modes and it wasn't even building for several months (April->late July).

PPC and a29k were both 32 bit ports and while they did promotions, I would hazard a guess the alpha was actually more sensitive to this stuff. Which suggests a possible path forward.

I can bootstrap & regression test alpha using QEMU user mode emulation. So we might be able to trigger something that way. It'll take some time, but might prove fruitful.


Jeff

Reply via email to