"H.J. Lu" <hjl.to...@gmail.com> writes: > On Tue, Dec 10, 2013 at 10:26 AM, Richard Sandiford > <rdsandif...@googlemail.com> wrote: >> "H.J. Lu" <hjl.to...@gmail.com> writes: >>> On Tue, Dec 10, 2013 at 9:57 AM, Richard Sandiford >>> <rdsandif...@googlemail.com> wrote: >>>> "H.J. Lu" <hjl.to...@gmail.com> writes: >>>>> On Tue, Dec 10, 2013 at 8:05 AM, Kirill Yukhin >>>>> <kirill.yuk...@gmail.com> wrote: >>>>>> On 09 Dec 14:08, H.J. Lu wrote: >>>>>>> >>>>>>> There are no regressions on Linux/x86-64 with -m32 and -m64. >>>>>>> Can you check if it improves code quality on x886? >>>>>> >>>>>> As second thought. If Tejas and Richard are right and it is simply >>>>>> incorrect >>>>>> to check any offsets in this hook, may be we can end up with patch in the >>>>>> bottom? >>>>> >>>>> What is wrong to pass the correct offset to >>>>> CANNOT_CHANGE_MODE_CLASS? Backends are free to >>>>> ignore it. >>>> >>>> The point is that: >>>> >>>>>> - /* Vector registers do not support subreg with nonzero offsets, >>>>>> which >>>>>> - are otherwise valid for integer registers. Since we can't see >>>>>> - whether we have a nonzero offset from here, prohibit all >>>>>> - nonparadoxical subregs changing size. */ >>>>>> - if (GET_MODE_SIZE (to) < GET_MODE_SIZE (from)) >>>>>> - return true; >>>> >>>> seems to be trying to reject things like (subreg:SF (reg:V4SF X) 1), >>>> which is always invalid for a single-register V4SF. See: >>> >>> That is correct. >> >> Sorry, what I mean is: that subreg is always invalid for single- >> register V4SFs regardless of the target. This isn't something that >> CANNOT_CHANGE_MODE_CLASS should be expected to check. >> > > Why is > > (define_insn "*movv4sfdi_subreg" > [(set (match_operand:DI 0 "nonimmediate_operand" "=rxm") > (subreg:DI (match_operand:V4SF 1 "register_operand" "x") 0))] > > invalid?
Sorry, I don't understand. I never said it was invalid. I said (subreg:SF (reg:V4SF X) 1) was invalid if (reg:V4SF X) represents a single register. On a little-endian target, the offset cannot be anything other than 0 in that case. So the CANNOT_CHANGE_MODE_CLASS code above seems to be checking for something that is always invalid, regardless of the target. That kind of situation should be rejected by target-independent code instead. In other words I'm arguing against the idea of passing the offset to CANNOT_CHANGE_MODE_CLASS (which you seemed to be supporting in the quote above). I think Kirill's patch to remove the i386.c check was the right way to go. There's no need for a separate insn though. Once you allow the subregs (as per Kirill's patch), the normal move patterns will handle them. Thanks, Richard
