On Mon, Sep 23, 2019 at 06:56:12PM +0100, Jozef Lawrynowicz wrote: > > It could have just done that as > > > > (set (reg:PSI 28) > > (zero_extend:PSI (reg:QI 12))) > > > > as far as I can see? Do you already have a machine pattern that matches > > that? > > Yes that combination is what I was expecting to see. I guess since char is > unsigned, a zero extend is needed to widen it, and then for the following > insn a > sign extend is added to widen the HImode integer.
Yeah, but a sign_extend:M1 of a zero_extend:M2 of an M3 (with M2>M3) is just a zero_extend:M1 of that M3. Somehow combine (or simplify-rtx) missed that; or come to think of it, it probably does that for MODE_INT things, but this is a MODE_PARTIAL_INT. Hrm, would it be correct then. I think it is? M2 is a normal MODE_INT, all bits in M2 are defined, the M2 value always is non-negative, the sign_extend:M1 always is the same as the zero_extend:M1 would be. > There isn't currently a pattern which matches that, but adding it > doesn't fix the issue which is why I thought it might need to be fixed earlier > in RTL generation. > Here is the pattern that is missing: > > +(define_insn "movqipsi" > + [(set (match_operand:PSI 0 "register_operand" "=r,r") > + (zero_extend:PSI (match_operand:QI 1 "general_operand" "rYs,m")))] > + "msp430x" > + "@ > + MOV.B\t%1, %0 > + MOV%X1.B\t%1, %0" > +) > + > > So will combine potentially be able to do away with (sign_extend > (zero_extend)) > in certain situations? Yes. > I filed PR/91865 which has all the relevant details from this thread. Thanks! > I can add the following nameless insn and combine will catch this case and > generate the better, smaller code: > > +(define_insn "*movqihipsi" > + [(set (match_operand:PSI 0 "register_operand" "=r,r") > + (sign_extend:PSI (zero_extend:HI (match_operand:QI 1 "general_operand" > "rYs,m"))))] > + "msp430x" > + "@ > + MOV.B\t%1, %0 > + MOV%X1.B\t%1, %0" > +) Good to hear that already works! combine should come up with the simpler formulation though, let me see what I can do. Segher
