On Thu, Nov 12, 2026 at 11:31:37PM -0700, Jeff Law wrote: > > > On 11/12/26 3:53 PM, [email protected] wrote: > > From: Trevor Saunders <[email protected]> > > > > gen_<name> still returns the pattern, or an rtx_insn * when the expand > > emits a sequence. gen_insn_<name> builds the uninserted insn with > > make_*_insn_raw. > I wouldn't be surprised if these assumptions are buried all over the > expansion code.
yeah, especially fun how its valid to do things like emit_call_insn (<jump insn>) in which case you might as well have just called emit_insn. So plenty of more room to rework how this works but stuff for next stage 1 at this point. > > > > This originally started with trying to return rtx_insn * from gen_* > > only when it was generating a sequence of instructions so would be a > > trivial return type change. However since plenty of target code > > interchanges function pointers to gen_foo that generates a sequence with > > one that only generates a pattern, I ended up creating a new set of > > gen_insn_foo as well that returns the same sequence, or for a pattern > > also generates the appropriate insn/call_insn/jump_insn wrapper for it, > > that ay gen_insn_foo consistently returns rtx_insn * in a state that can > > be passed to emit_*. ultimately I tend to think it may make sense to > > switch more to the gen_insn_foo form on the theory its slightly faster > > if the insn type is statically computed rather than emit_* calling > > classify_insn at compile time. However I'm not sure if there is > > general agreement with that direction, or other ideas? As can be seen in > > the s390.cc changes this removes a few as_a casts and once more fully > > converted to gen_insn_* helps more with that? > > > > bootstrapped and regtested on ppc64le-linux-gnu, and run through > > config-list.mk with the following patch, ok? > > > > Thanks > > > > Trev > > > > gcc/ChangeLog: > > > > * emit-rtl.cc (make_insn_raw, make_debug_insn_raw, > > make_jump_insn_raw, make_call_insn_raw): Clear the next and > > previous links. Export the jump and call makers. > > * genemit.cc (start_gen_insn): Take return_pat. > > (gen_insn_wrapper): New function. > > (gen_insn, gen_expand): Emit a gen_insn_NAME wrapper. > > * genflags.cc (gen_proto): Declare gen_insn_NAME, stub an elided > > pattern, and return rtx_insn * from a sequence expand. > > * gensupport.cc (get_make_insn_function): Rename from > > get_emit_function. Return make_*_insn_raw. > > * gensupport.h (get_make_insn_function): Declare. > > * rtl.cc (classify_insn_vec, predicate_is_always_insn): New > > functions. > > (classify_insn): Classify PARALLEL and MATCH_PARALLEL from their > > elements. Treat a const_int match_operand as an insn. > > * rtl.h (make_jump_insn_raw, make_call_insn_raw): Declare. > > * config/i386/i386-expand.cc: Use an rtx_insn * generator. > > * config/aarch64/aarch64-builtins.cc > > (aarch64_expand_builtin_rsqrt): Use an rtx_insn * generator. > > * config/arm/arm.cc (arm_block_set_unaligned_vect): Likewise. > > * config/avr/avr-passes.cc: Likewise. > > * config/avr/avr-protos.h (avr_emit3_fix_outputs): Likewise. > > * config/avr/avr.cc (avr_emit3_fix_outputs): Likewise. > > * config/i386/i386-protos.h (ix86_expand_reduc): Likewise. > > * config/i386/i386.md: Likewise. > > * config/i386/mmx.md: Likewise. > > * config/i386/sse.md: Likewise. > > * config/i386/sync.md: Likewise. > > * config/ia64/ia64.cc: Likewise. > > * config/loongarch/lasx.md: Likewise. > > * config/loongarch/loongarch-protos.h: Likewise. > > * config/loongarch/loongarch.cc: Likewise. > > * config/mips/loongson-mmi.md: Likewise. > > * config/mips/mips-protos.h: Likewise. > > * config/mips/mips.cc: Likewise. > > * config/rl78/rl78-protos.h: Likewise. > > * config/rl78/rl78.cc: Likewise. > > * config/rs6000/rs6000-string.cc: Likewise. > > * config/rs6000/rs6000.cc: Likewise. > > > > Signed-off-by: Trevor Saunders <[email protected]> > So the guiding principle is to make the code more type safe, so I think that > inherently means we have to tackle this problem at some point. If seems > like ultimately something has to be done to split the API in two. A quick LLM investigation suggested that allmost all gen_<foo> callers immediately pass the result to emit_* constructing the insn wrapper if only a pattern is returned from gen_foo, so I wonder if we can switch entirely to have the generated code return a complete insn not just the pattern. A comment in genemit said returning just the pattern was to save memory but that concern doesn't make much sense given the caller per above allocates that extra memory for the insn in almost all cases anyway? or am I missing something? > WIth that in mind OK. But perhaps wait a few days to give others a chance > to chime in. yeah, definitely looking for thoughts here! thanks Trev
