On Thu, Mar 12, 2026 at 5:29 PM Vineet Gupta <[email protected]> wrote: > > On 3/12/26 1:45 PM, H.J. Lu wrote: > > On Thu, Mar 12, 2026 at 10:46 AM Vineet Gupta <[email protected]> > > wrote: > >> Hi, > >> > >> I wanted some insight/clarity on subreg promotion at expand time > >> following a promote_function_mode() > >> Apologies Roger and HJ for explicit CC but it seems you touched the same > >> general area in 2021 and 2026 respectively. > >> > >> Here's my understand of various pieces and the problem I'm running into. > >> > >> 1a. PROMOTE_MODE has no direct ABI implications on its own but could do > >> so indirectly if TARGET_PROMOTE_FUNCTION_MODE uses the default always > >> which in turn uses this macro. > >> > >> 1b. According to docs [1] PROMOTE_MODE for for ISAs supporting only > >> 64-bit registers would define it to word_mode (64) and I'm implying > >> further that for ISAs supporting both it should be OK to define either > >> 32 or 64 although it might be desirable to have 32, just for codegen > >> fiddling with fewer bits if nothing else. > >> > >> "On most RISC machines, which only have operations that operate on a > >> full register, > >> define this macro to set m to word_mode if m is an integer mode > >> narrower than > >> BITS_PER_WORD...." > >> > >> The RISC-V implementation is textbook perfect: for rv64 it would promote > >> anything smaller that DI to DI (since that's how wide the container > >> itself is) and if SI clear the unsigned bit as well since most ALU > >> operations would sign extend the 32-bit result to 64-bits. > >> > >> BPF currently defines it to promote anything smaller that DI to DI: that > >> might be a bit conservative and lead to fewer 32-bit only insns. A > >> future/separate change to promote anything smaller than SI to SI can be > >> done later and would not be wrong. > >> > >> 2a. Does setting SUBREG_PROMOTED_VAR_P imply that rest of pass pipeline > >> assumes it is already promoted (thus potentially eliding any subsequent > >> zero/sign extensions) or does it ensure that such extensions will > >> always be generated on any moves. The documentation [2] seems to suggest > >> it is the latter, although usage in the code seems to be more like former. > >> > >> "Nonzero in a subreg if it was made when accessing an object that > >> was promoted > >> to a wider mode in accord with the PROMOTED_MODE machine description > >> macro > >> ... . In this case, the mode of the > >> subreg is the declared mode of the object and the mode of SUBREG_REG > >> is the > >> mode of the register that holds the object. *Promoted variables are > >> always either > >> sign- or zero-extended to the wider mode on every assignment*. > >> Stored in the > >> in_struct field and printed as ‘/s’." > >> > >> FWIW my patch [3] removed code which was clearing SUBREG_PROMOTED_VAR_P > >> as it was leading to extraneous sign extensions on RISC-V.So I tend to > >> think that keeping subreg promoted prevents subsequent generation of > >> extensions, > >> > >> 2023-10-16 8eb9cdd14218 expr: don't clear SUBREG_PROMOTED_VAR_P flag > >> for a promoted subreg [target/111466] > >> > >> > >> 2b. expand_call () depending on modes of @target and @rettype would call > >> ABI promotion for return value, wrap target in a subreg with new mode > >> and set SUBREG_PROMOTED_VAR_P eagerly. > >> > >> store_expr > >> |- expand_call > >> if REG_P(target) && GET_MODE (target) != TYPE_MODE > >> (rettype)) > >> ... > >> pmode = promote_function_mode (type, ret_mode, > >> &unsignedp, funtype, 1); > >> target = gen_lowpart_SUBREG (ret_mode, target); > >> SUBREG_PROMOTED_VAR_P (target) = 1; > >> SUBREG_PROMOTED_SET (target, unsignedp); > >> > >> The followig convert_move (@from as subreg) just strips off the subreg > >> ... > >> |- convert_move > >> if (GET_CODE (from) == SUBREG > >> && SUBREG_PROMOTED_VAR_P (from) > >> ... > >> from = gen_lowpart (to_int_mode, SUBREG_REG (from)); > >> > >> This supposedly ensures that extensions won't be generated ? but between > >> setting the subreg promoted and stripping the outer, an extension was > >> not generated for return anyways, what am I missing ? > >> > >> 3. The reason for the questions above is PR/124171 [4] where we need to > >> change gcc BPF function ABI to promote arguments as well as return > >> values both in callee. I'm guessing the last part is atypical as args > >> promoted in caller would imply return promoted in callee - but BPF code > >> could be called from as well as calling into other ABIs, such as x86 > >> kernel code and thus needs to ensure sanity in either direction. > >> > >> For implementing this > >> > >> * I'm specifying TARGET_PROMOTE_FUNCTION_MODE to default > >> promote_always: so both args and retval will be promoted. > >> * Currently bpf PROMOTE_MODE defaults to promoting anything smaller > >> than DI to DI (although ISA has insn to do SI mode only ops, and it > >> could be changed to that effect later on, separately, but that is > >> not really needed for the ABI change) > >> > >> This work for most part, except for a single weird test which fails to > >> promote a bool return value in caller. > >> > >> _Bool bar_bool(void); > >> int foo_bool_ne1(void) { > >> if (bar_bool() != 1) return 0; else return 1; > >> } > >> > >> On trunk this generates > >> > >> foo_bool_ne1: > >> call bar_bool > >> r0 &= 0xff > >> exit > >> > >> With TARGET_PROMOTE_FUNCTION_MODE to default always (and unchanged > >> PROMOTE_MODE: sub DI to DI), it generates > >> > >> foo_bool_ne1: > >> call bar_bool > >> r0 = (s32) 0xff > >> exit > >> > >> The s32 truncation doesn't seem right as it needs to clamp it to 8 bits > >> (ideally 1 but bool for most targets is implemented as QI). > >> > > Please try this enclosed patch. > > Awesome, it does work perfectly for the tripping tests before. I'll give > it more extensive testing but this looks promising already.
If it works, please update the comment to explain that it is done for compatibility reasons. > FWIW, the newly wired up default_promote_function_mode_sign_extend () > has the same semantics as updated PROMOTE_MODE so in theory we could get > away with keeping bpf tgt hook to > default_promote_function_mode_always_promote () but former conveys the > semantics better. > > Thx, > -Vineet -- H.J.
