This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH v2] combine: Improve change_zero_ext, call simplify_set afterwards.
On Thu, Dec 22, 2016 at 04:18:34PM +0100, Georg-Johann Lay wrote:
> >>>If you don't have instruction scheduling subregs of mem are allowed (and
> >>>are counted as registers). Combine asks recog, and it think this is
> >>>fine.
> >>>
> >>>Why does reload use r31 here? Why does it think that is valid for
> >>>HImode?
> >>
> >>I can't tell you, all I'm seeing bunch of new FAILs after r243578.
The avr port does not define CANNOT_CHANGE_MODE_CLASS. It probably
should.
> >>i.e. the paradoxical subreg could be resolved somehow and R25 is
> >>uninitialized. It this the purpose of the combine change to come
> >>up with uninitialized regs because it is known that just the
> >>initialized parts are used by the code?
The purpose of the combine change is to write widening extracts in a
more general form, so that backends for processors that can do such
more general things do not have to write hundreds (literally) extra
patterns for all the cases that could be written as zero_extract.
> >>(insn 98 97 58 9 (set (reg:QI 31 r31)
> >> (mem:QI (reg:HI 30 r30) [1 *operands_17(D)+0 S1 A8]))
> >>"pr26833.c":11 56 {movqi_insn}
> >> (nil))
> >>(jump_insn 58 98 59 9 (set (pc)
> >> (if_then_else (eq (and:HI (reg:HI 31 r31)
> >> (const_int 1 [0x1]))
> >> (const_int 0 [0]))
> >> (label_ref 70)
> >> (pc))) "pr26833.c":11 415 {*sbrx_and_branchhi}
> >> (int_list:REG_BR_PROB 375 (nil))
> >> -> 70)
> >>
> >>insn 98 is valid but overrides parts of HI:30 set by insn 97.
> >>
> >>As it appears, the QI memory is loaded to R31 which is valid, and
> >>then reload drops in that register as replacement for the paradoxical
> >>subreg and comes up with HI:31 in insn 58.
> >>
> >>Actually a zero-extend would be needed, does it?
The AND clears the top bits already.
> >Please correct me if I'm wrong, but I see no bugs in the combined
> >expressions.
>
> Algebraically it looks correct, but I am unsure if reload is supposed
> to handle such situations.
It is valid RTL. reload should handle it.
> And I must admit I never saw "paradoxical"
> extractions before; I would expected an extension in that case, not an
> extraction.
An extension _of an extraction_. Widening extractions are valid (and I
didn't see them until a few weeks ago either, heh -- that's why Dominik
needed this patch ;-) )
Segher