On Fri, Jan 13, 2017 at 10:20:58PM +1030, Alan Modra wrote: > Rather than using unspecs in doloop insns to stop combine creating > these insns, use legitimate_combined_insn. > > I'm not sure why the original patch implementing > legitimate_combined_insn did not store the result of recog to the insn > but it seems good to me, and would allow the recog call in > ix86_legitimate_combined_insn to be omitted. (I tested that too, not > shown here.)
It is always restored after the hook call, so this change is fine. > + /* Reject creating doloop insns. Combine should not be allowed > + to create these for a number of reasons: It also prevents combine from combining any instruction into an existing doloop insn (resulting in a doloop insn again). This isn't too serious, almost always this is just a register copy that will be optimised away some other way. The unspec hack has more significant problems. > + 3) Faced with not being able to allocate ctr for ctrsi/crtdi > + insns, the register allocator sometimes uses floating point > + or vector registers for the pseudo. Since ctrsi/ctrdi is a > + jump insn and output reloads are not implemented for jumps, > + the ctrsi/ctrdi splitters need to handle all possible cases. > + That's a pain, and it gets to be seriously difficult when a > + splitter that runs after reload needs memory to transfer from > + a gpr to fpr. See PR70098 and PR71763 which are not fixed > + for the difficult case. It's better to not create problems > + in the first place. */ Yeah :-) Note that the problems still can happen, just much less frequently. > @@ -12751,9 +12749,8 @@ (define_expand "ctr<mode>" > ;; JUMP_INSNs. > ;; For the length attribute to be calculated correctly, the > ;; label MUST be operand 0. > -;; The UNSPEC is present to prevent combine creating this pattern. Maybe add a note here that rs6000_legitimate_combined_insn will refuse to combine to this? Okay for trunk. Thanks! Segher
