On 6/6/19 10:06 AM, Richard Earnshaw (lists) wrote: > On 06/06/2019 15:55, Fredrik Hederstierna wrote: >>> From: Segher Boessenkool <seg...@kernel.crashing.org> >>> Sent: Thursday, June 6, 2019 4:02 PM >>> To: Richard Earnshaw (lists) >>> Cc: Jeff Law; Fredrik Hederstierna; gcc@gcc.gnu.org >>> Subject: Re: ARM peephole2 from 2003 never merged, still valid >> >>> That doesn't stop combine from considering it. It does make that first SET >>> survive, so that you get a parallel as final insn. It may not like that >>> one of the parallel SETs is just a move. Needs testcase :-) >> >> Hi all, thanks for investigating this, >> I added semi-stripped testcase in original issue taken from CSiBE teem >> sources >> >> See attachment in >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=9663 >> >> Tested gcc-9.1.0 for ARM 32bit targets, first without peephole2 patch: >> >> 00000000 <nrrdRangeSet>: >> 0: e92d407f push {r0, r1, r2, r3, r4, r5, r6, lr} >> 4: e2504000 subs r4, r0, #0 >> 8: 0a00003f beq 10c <nrrdRangeSet+0x10c> >> c: e3510000 cmp r1, #0 >> 10: e1a05001 mov r5, r1 >> >> then with new peephole2 patch: >> >> 00000000 <nrrdRangeSet>: >> 0: e92d407f push {r0, r1, r2, r3, r4, r5, r6, lr} >> 4: e2504000 subs r4, r0, #0 >> 8: 0a00003e beq 108 <nrrdRangeSet+0x108> >> c: e2515000 subs r5, r1, #0 >> >> Thanks, Fredrik >> > > The reason combine doesn't catch this is because at the time it runs the > MOV is in a different basic block. Later on it is sunk into the same > basic block, but it's then too late to do the merge. And while this looks well suited for compare-elim, nobody's done the work to enable compare-elim on ARM. It may also be the case that compare-elim is still too early, at least for the testcase in the PR.
So should we just go with the peephole2? jeff
