Andreas Krebbel via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > On 1/14/22 20:41, Andreas Krebbel via Gcc-patches wrote: >> On 1/14/22 08:37, Richard Biener wrote: >> ... >>> Can the gist of this bug be put into the GCC bugzilla so the rev can >>> refer to it? >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104034 >> >>> Can we have a testcase even? >> The testcase from Jakub is in the BZ. However, since it doesn't fail with >> head I didn't try to >> include it in my patch. >> >>> I'm not quite understanding the problem but is it that, say, >>> >>> (subreg:DI (reg:V2DI ..) 0) >>> >>> isn't the same as >>> >>> (lowpart:DI (reg:V2DI ...) 0) >> >> (reg:DI v0) does not match the lower order bits of (reg:TI v0) >> >>> ? The regcprop code looks more like asking whether the larger reg >>> is a composition of multiple other hardregs and will return the specific >>> hardreg corresponding to the lowpart - so like if on s390 the vector >>> registers overlap with some other regset. But then doing the actual >>> accesses via the other regset regs doesn't actually work? Isn't the >>> backend then lying to us (aka the mode_change_ok returns the >>> wrong answer)? >> >> can_change_mode_class should do the right thing. We return false in case >> somebody wants to change TI >> to DI for a vector register. However, the hook never gets called like this >> from regcprop. regcprop >> only asks whether it is ok to change (reg:TI r8) to (reg:DI r8) and that's >> indeed ok. > > After writing this I'm wondering whether this would be a better fix: > > diff --git a/gcc/regcprop.c b/gcc/regcprop.c > index 18132425ab2..b6a3f4e3804 100644 > --- a/gcc/regcprop.c > +++ b/gcc/regcprop.c > @@ -402,7 +402,8 @@ maybe_mode_change (machine_mode orig_mode, machine_mode > copy_mode, > > if (orig_mode == new_mode) > return gen_raw_REG (new_mode, regno); > - else if (mode_change_ok (orig_mode, new_mode, regno)) > + else if (mode_change_ok (orig_mode, new_mode, regno) > + && mode_change_ok (copy_mode, new_mode, copy_regno)) > { > int copy_nregs = hard_regno_nregs (copy_regno, copy_mode); > int use_nregs = hard_regno_nregs (copy_regno, new_mode); >
Yeah, this looks good to me FWIW. Richard
