On Thu, 2020-01-30 at 17:54 +0000, Richard Sandiford wrote: > Jeff Law <l...@redhat.com> writes: > > On Wed, 2020-01-29 at 19:18 +0000, Richard Sandiford wrote: > > > Andreas Schwab <sch...@suse.de> writes: > > > > On Jan 27 2020, Richard Sandiford wrote: > > > > > > > > > * simplify-rtx.c (simplify_truncation): Extend sign/zero_extract > > > > > simplification to handle subregs as well as bare regs. > > > > > > > > That breaks gcc.target/m68k/pr39726.c > > > > > > Gah. Jeff pointed out off-list that it also broke > > > gcc.target/sh/pr64345-1.c on sh3-linux-gnu. It didn't look like either > > > of them would be fixable in an acceptably non-invasive and unhacky way, > > > so I've reverted the patch "for now". > > I would have considered letting those two targets regress those tests > > to move forward on 87763. aarch64 is (IMHO) more important than the sh > > and m68k combined ;-) It also seems to me that your patch generates > > better RTL and that we could claim that a port that regresses on code > > quality needs its port maintainer to step in and fix the port. > > > > WRT the m68k issue I'd think it could be fixed by twiddling > > cbranchsi4_btst_reg_insn_1 to accept a mode iterator on the > > zero_extract and making some minor adjustments in its output code. > > Something like the attached. I haven't tested it in any real way and > > haven't really thought about whether or not it does the right thing for > > a MEM operand. > > It just feels like this is breaking the contract with extv and extzv, > where all *_extracts are supposed to be in the mode that those patterns > accept. My i386 patch was doing that too TBH, I was just being > optimistic given that QImode was already handled by that pattern. :-) > > So IMO my patch has too many knock-on effects for it to be suitable for > stage 4. While we have make_extraction, we probably have to leave this > kind of expression untouched. > > For AArch64 I was planning on adding a new pattern to match the > (subreg:SI (zero_extract:DI ...)) -- as one of the comments in the > PR suggested -- but with the subreg matched via subreg_lowpart_operator, > to make it endian-safe. This is similar in spirit to the new i686 > popcount patterns. Fair enough on the simplify-rtx change :-) One might argue that we should be loosening the requirements, but memory operands are particularly troublesome. I think HP and I had a light discussion on that a year or so ago.
WRT adding patterns to aarch64. Yea, I went that route last year, but never was able to go from "hey, this seems to work pretty well" to "it's OK for the trunk". Jeff
