> FWIW, I suppose the comment above the check would need to be updated as well. > > It sounds like the comment was anticipating that no > riscv_regmode_natural_size check would be needed after the fix, but perhaps I > misunderstood.
Indeed, it needs updating. I intended to change this once the patch behaves reasonably but then of course forgot to update. > I'd prefer to remove the assert here. I'm not sure what purpose > it serves. There are multiple valid reasons for simplify_gen_subreg > to return null and extract_bit_field is the correct fallback when > it does. extract_bit_field will itself assert if the combination > is really badly formed. Glad you said that :) My initial hunch was exactly to delete it but as I couldn't really figure out why it's there I rather kept it. > Not sure I understand this one. Could you go into more detail? > Also, why is this keyed off y_inner being nonnull? Does the non-subreg > case work correctly? What about the case where y is a subreg but the > inner register is a different size (and thus y_inner is null)? Initially I had Y instead of y_inner (in the comment still) but what actually goes wrong is (subreg:OI (reg:V4DI)). OImode can be split into DImode so we later try (subreg:DI (subreg:OI (reg:V4DI))) = (subreg:DI (reg:V4DI)) which fails. Maybe it's better to strip the subreg and then check, but keep the mode check on the outer mode. > > The "memory path below" just seems to "validize" the memories and > still falls through to emit_move_insn_1 in the end. Yeah I think I just spotted the constant handling without realizing it only deals with constants and hoped for the best :/ > It's not obvious to me how safe it is to pull out at this stage. > Did you consider making interesting_mode_p return false instead? Yes, that was my initial try. However I hit an issue with pr44136.c where simplify_subreg_concatn asserts that either outermode or innermode are interesting_mode_p. I believe OP was a debug const vector. I'll try handling this more gracefully while keeping interesting_mode_p = false. -- Regards Robin
