On 01/11/2018 11:32 AM, Jakub Jelinek wrote: > Hi! > > As mentioned in this PR, we have effectively: > (set (reg2) (expression)) > ... > (set (reg1) (reg2)) > ... > (set (reg2) (any_extend (reg1))) > in pr50083.c with -O2 -m32, where the expression in this case is actually > a QImode memory read has any_extend is zero extension to SImode. > Currently REE tries to change that to just > (set (reg2) (any_extend (reg2))) > ... > (set (reg1) (reg2)) > which for one isn't useful, and fails because reg2 is %si which doesn't have > a QImode subreg in 32-bit code. Right. And in fact, the whole reason why we get the code we do is because reg2 can't be used in QImode.
This patch instead sees the copying of > the reg2 to reg1 as def_insn and in that case looks further for the defining > insn of that (def_insn2) and transforms it thus to: > (set (reg2) (any_extend (expression)) > ... > (set (reg1) (reg2)) // This insn is untouched, stays in the > // mode it used before, but can be DCEd > // later if the reg isn't really used > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Right. > > No testcase included, as it fixes an existing one: > -FAIL: gcc.target/i386/pr50038.c scan-assembler-times movzbl 2 (found 3 times) > > 2018-01-11 Jakub Jelinek <ja...@redhat.com> > > PR target/82682 > * ree.c (combine_reaching_defs): Optimize also > reg2=exp; reg1=reg2; reg2=any_extend(reg1); into > reg2=any_extend(exp); reg1=reg2;, formatting fix. It seems reasonable. Your patch might even allow ree.c to pick up secondary effects. Totally agree with the decision to leave the copy in the IL, we already do the same for other cases handled by ree.c OK. jeff
