"Robin Dapp" <[email protected]> writes:
>> That doesn't look right. The CONCATNs are generated by the pass
>> itself
>> and can't be left as-is. The failure path for !resolve_subreg_p is
>> also different from the failure path for !simplify_subreg_concatn.
>>
>> So it feels like the problem is elsewhere to me.
>
> Yes, agreed.
>
> What we're decomposing is
>
> (insn 4 3 5 2 (set (subreg:DI (reg:TI 137) 0)
> (subreg:DI (reg:TI 138) 0)) "pr44136.c":20:1 275 {*movdi_64bit}
> (nil))
> (insn 5 4 39 2 (set (subreg:DI (reg:TI 137) 8)
> (subreg:DI (reg:TI 138) 8)) "pr44136.c":20:1 275 {*movdi_64bit}
> (nil))
> (debug_insn 39 5 38 2 (var_location:V4SF D#4 (subreg:V4SF (reg:TI 137) 0)) -1
> (nil))
>
> Neither (subreg:V4SF (reg:TI)) nor (subreg:V4SF (concatn [(reg:DI)
> (reg:DI)])) look harmful from a regmode-natural-size point of view.
>
> Therefore, I wonder if the interesting_mode_p checks in
> simplify_subreg_concatn aren't over-eager now that we restricted them
> further. simplify_subreg_concatn doesn't even try splitting but just
> builds a new subreg for which we only need to verify that the mode sizes
> are constant.
>
> As we filter out the problematic cases before, my try would be to just
> return NULL like:
>
> @@ -649,16 +666,19 @@ decompose_register (unsigned int regno)
> static rtx
> simplify_subreg_concatn (machine_mode outermode, rtx op, poly_uint64
> orig_byte)
> {
> - unsigned int outer_size, outer_words, inner_size, inner_words;
> + unsigned int outer_size, inner_size;
> machine_mode innermode, partmode;
> rtx part;
> unsigned int final_offset;
> unsigned int byte;
>
> innermode = GET_MODE (op);
> - if (!interesting_mode_p (outermode, &outer_size, &outer_words)
> - || !interesting_mode_p (innermode, &inner_size, &inner_words))
> - gcc_unreachable ();
> +
> + if (!GET_MODE_SIZE (innermode).is_constant (&inner_size))
> + return NULL_RTX;
> + if (!GET_MODE_SIZE (outermode).is_constant (&outer_size)
> + || !orig_byte.is_constant ())
> + return NULL_RTX;
I think the inner_size part does still make sense as:
if (!interesting_mode_p (innermode, &inner_size, &inner_words))
gcc_unreachable ();
But I agree that it's now the wrong check for the outer_size check
(and I suppose was always a false abstraction).
If outermode is constant size then orig_byte must be too.
The comment in:
/* Must be constant if interesting_mode_p passes. */
byte = orig_byte.to_constant ();
would need to be updated. So I guess that makes it:
if (!interesting_mode_p (innermode, &inner_size, &inner_words))
gcc_unreachable ();
if (!GET_MODE_SIZE (outermode).is_constant (&outer_size))
return NULL_RTX;
/* Must be constant if outer_size is. */
byte = orig_byte.to_constant ();
LGTM with that variation.
Thanks,
Richard
