[llvm] r190309 - [ARMv8] Prevent generation of deprecated IT blocks on ARMv8 in Thumb mode.

[Hal Finkel]

----- Original Message -----
> From: "Hal Finkel" <hfinkel at anl.gov>
> To: "Artyom Skrobov" <Artyom.Skrobov at arm.com>
> Cc: "Joey Gouly" <Joey.Gouly at arm.com>, llvm-commits at cs.uiuc.edu, "Tim Northover" <t.p.northover at gmail.com>
> Sent: Wednesday, November 27, 2013 3:32:55 PM
> Subject: Re: [llvm] r190309 - [ARMv8] Prevent generation of deprecated IT	blocks	on ARMv8 in Thumb mode.
> 
> ----- Original Message -----
> > From: "Artyom Skrobov" <Artyom.Skrobov at arm.com>
> > To: "Hal Finkel" <hfinkel at anl.gov>
> > Cc: "Joey Gouly" <Joey.Gouly at arm.com>, llvm-commits at cs.uiuc.edu,
> > "Tim Northover" <t.p.northover at gmail.com>
> > Sent: Wednesday, November 27, 2013 1:49:13 PM
> > Subject: RE: [llvm] r190309 - [ARMv8] Prevent generation of
> > deprecated IT	blocks	on ARMv8 in Thumb mode.
> > 
> > Hello Hal,
> > 
> > >> I'm attaching a new patch which reverts
> > >> IfConverter::ScanInstructions
> > >> to its original logic (checking the predicated status of the
> > >> conditional branches) and implements
> > >> Thumb2InstrInfo::isPredicated
> > >> to reflect the somewhat unorthodox "conditional but not
> > >> predicated"
> > >> convention for the ARMv8/Thumb conditional branches.
> > >> 
> > >> Hal, can you confirm that this patch restores the correct
> > >> functioning
> > >> on PPC?
> > >
> > > I'll check.
> > >
> > > Why are you restoring this part?
> > >
> > >      if (BBI.ClobbersPred && !isPredicated) {
> > >        // Predicate modification instruction should end the block
> > >        (except for
> > >        // already predicated instructions and end of block
> > >        branches).
> > > +      if (isCondBr) {
> > > +        // A conditional branch is not predicable, but it may be
> > > eliminated.
> > > +        continue;
> > > +      }
> > > +
> > 
> > This enables conversion of fragments such as
> > 
> >         cmp     r12, r2
> >         bne     .LBB6_4
> > @ BB#2:
> >         cmp     r1, r3
> >         bne     .LBB6_4
> >         strexd  r5, lr, r4, [r0]
> > 
> > into
> > 
> >         cmp     r12, r2
> >         it      eq
> >         cmpeq   r1, r3
> >         bne     .LBB6_4
> > @ BB#2:
> >         strexd  r5, lr, r4, [r0]
> > 
> > As you can see, in this case, the last instruction of the branch
> > (cmp
> > r1, r3; bne .LBB6_4) is not predicable on the branch condition (the
> > original CPSR), but it's going to be eliminated during the
> > conversion, so its non-predicability doesn't matter.
> 
> Can you explain what this means? I still obviously see those
> instructions in the transformation result.
> 
> > 
> > 
> > In the meantime, have you been able to create a test case for the
> > invalid PPC code generation being caused by r190309 ?
> > 
> 
> I've certainly made a lot of progress on this. I've discussed some of
> this in the e-mail(s) that I sent to you on Monday. I could
> certainly appreciate your comments on those. I will continue to work
> on creating a useful .ll input that demonstrates the problem.

In case it helps, I've attached what I have so far. This can almost certainly be reduced further (at least by getting rid of the structure types). Nevertheless, unfortunately, the process is generally slow because the test case is fragile: getting a difference to appear at all due to r190309 is fragile, and getting the transformation to be invalid is even more fragile.

 -Hal

> 
> Thanks again,
> Hal
> 
> > 
> > 
> > 
> > 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory
-------------- next part --------------
A non-text attachment was scrubbed...
Name: r190309-test.ll
Type: application/octet-stream
Size: 38678 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131127/112c8a0b/attachment.obj>