[llvm] r322373 - [PowerPC] Don't miscompile rotate+mask into an ANDIo if it can't recreate the immediate
Nemanja Ivanovic via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 12 09:03:26 PST 2018
Hi Ben,
thanks for committing this fix. You're absolutely right - the code was
incorrect as the ANDIo does not materialize a new constant.
I know it seems like this transform is useless because it doesn't get rid
of any instructions. However, the CR-setting rotate instructions are
cracked instructions with double the total latency of ANDIo.
I'll post a follow-up on Phabricator. I think the complete fix should be:
- If the defining load-immediate has a single use, just update the
immediate operand of the LI/LI8
- If the GPR result isn't needed, give the ANDIo an immediate that is
either zero or the original immediate (to ensure the correct CR-bit is set)
- Otherwise (and in the late pass) use your fix
Thanks again and sorry about the issues this caused,
Nemanja
On Fri, Jan 12, 2018 at 4:07 PM, Benjamin Kramer <benny.kra at gmail.com>
wrote:
> Nemanja, can you review this? I didn't want to disable the entire
> transform again, but still avoid a miscompile that's triggered by it.
>
> On Fri, Jan 12, 2018 at 4:03 PM, Benjamin Kramer via llvm-commits
> <llvm-commits at lists.llvm.org> wrote:
> > Author: d0k
> > Date: Fri Jan 12 07:03:24 2018
> > New Revision: 322373
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=322373&view=rev
> > Log:
> > [PowerPC] Don't miscompile rotate+mask into an ANDIo if it can't
> recreate the immediate
> >
> > I'm not even sure if this transform is ever worth it, but this at least
> > stops the bleeding.
> >
> > Modified:
> > llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp
> > llvm/trunk/test/CodeGen/PowerPC/convert-rr-to-ri-instrs.mir
> >
> > Modified: llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/
> PowerPC/PPCInstrInfo.cpp?rev=322373&r1=322372&r2=322373&view=diff
> > ============================================================
> ==================
> > --- llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp (original)
> > +++ llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp Fri Jan 12 07:03:24
> 2018
> > @@ -2444,6 +2444,8 @@ bool PPCInstrInfo::convertToImmediateFor
> > Is64BitLI = Opc != PPC::RLDICL_32;
> > NewImm = InVal.getSExtValue();
> > SetCR = Opc == PPC::RLDICLo;
> > + if (SetCR && (SExtImm & NewImm) != NewImm)
> > + return false;
> > break;
> > }
> > return false;
> > @@ -2471,6 +2473,8 @@ bool PPCInstrInfo::convertToImmediateFor
> > Is64BitLI = Opc == PPC::RLWINM8 || Opc == PPC::RLWINM8o;
> > NewImm = InVal.getSExtValue();
> > SetCR = Opc == PPC::RLWINMo || Opc == PPC::RLWINM8o;
> > + if (SetCR && (SExtImm & NewImm) != NewImm)
> > + return false;
> > break;
> > }
> > return false;
> >
> > Modified: llvm/trunk/test/CodeGen/PowerPC/convert-rr-to-ri-instrs.mir
> > URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> CodeGen/PowerPC/convert-rr-to-ri-instrs.mir?rev=322373&r1=
> 322372&r2=322373&view=diff
> > ============================================================
> ==================
> > --- llvm/trunk/test/CodeGen/PowerPC/convert-rr-to-ri-instrs.mir
> (original)
> > +++ llvm/trunk/test/CodeGen/PowerPC/convert-rr-to-ri-instrs.mir Fri Jan
> 12 07:03:24 2018
> > @@ -561,6 +561,16 @@
> > }
> >
> > ; Function Attrs: norecurse nounwind readnone
> > + define i64 @testRLDICLo2(i64 %a, i64 %b) local_unnamed_addr #0 {
> > + entry:
> > + %shr = lshr i64 %a, 11
> > + %and = and i64 %shr, 16777215
> > + %tobool = icmp eq i64 %and, 0
> > + %cond = select i1 %tobool, i64 %b, i64 %and
> > + ret i64 %cond
> > + }
> > +
> > + ; Function Attrs: norecurse nounwind readnone
> > define zeroext i32 @testRLWINM(i32 zeroext %a) local_unnamed_addr #0 {
> > entry:
> > %shl = shl i32 %a, 4
> > @@ -602,6 +612,15 @@
> > }
> >
> > ; Function Attrs: norecurse nounwind readnone
> > + define zeroext i32 @testRLWINMo2(i32 zeroext %a, i32 zeroext %b)
> local_unnamed_addr #0 {
> > + entry:
> > + %and = and i32 %a, 255
> > + %tobool = icmp eq i32 %and, 0
> > + %cond = select i1 %tobool, i32 %b, i32 %a
> > + ret i32 %cond
> > + }
> > +
> > + ; Function Attrs: norecurse nounwind readnone
> > define i64 @testRLWINM8o(i64 %a, i64 %b) local_unnamed_addr #0 {
> > entry:
> > %a.tr = trunc i64 %a to i32
> > @@ -3904,6 +3923,59 @@ body: |
> >
> > ...
> > ---
> > +name: testRLDICLo2
> > +# CHECK-ALL: name: testRLDICLo2
> > +alignment: 4
> > +exposesReturnsTwice: false
> > +legalized: false
> > +regBankSelected: false
> > +selected: false
> > +tracksRegLiveness: true
> > +registers:
> > + - { id: 0, class: g8rc, preferred-register: '' }
> > + - { id: 1, class: g8rc_and_g8rc_nox0, preferred-register: '' }
> > + - { id: 2, class: g8rc_and_g8rc_nox0, preferred-register: '' }
> > + - { id: 3, class: crrc, preferred-register: '' }
> > + - { id: 4, class: g8rc, preferred-register: '' }
> > +liveins:
> > + - { reg: '%x3', virtual-reg: '%0' }
> > + - { reg: '%x4', virtual-reg: '%1' }
> > +frameInfo:
> > + isFrameAddressTaken: false
> > + isReturnAddressTaken: false
> > + hasStackMap: false
> > + hasPatchPoint: false
> > + stackSize: 0
> > + offsetAdjustment: 0
> > + maxAlignment: 0
> > + adjustsStack: false
> > + hasCalls: false
> > + stackProtector: ''
> > + maxCallFrameSize: 4294967295
> > + hasOpaqueSPAdjustment: false
> > + hasVAStart: false
> > + hasMustTailInVarArgFunc: false
> > + savePoint: ''
> > + restorePoint: ''
> > +fixedStack:
> > +stack:
> > +constants:
> > +body: |
> > + bb.0.entry:
> > + liveins: %x3, %x4
> > +
> > + %1 = COPY %x4
> > + %0 = LI8 200
> > + %2 = RLDICLo %0, 61, 3, implicit-def %cr0
> > + ; CHECK-NOT: ANDI
> > + ; CHECK-LATE-NOT: andi.
> > + %3 = COPY killed %cr0
> > + %4 = ISEL8 %1, %2, %3.sub_eq
> > + %x3 = COPY %4
> > + BLR8 implicit %lr8, implicit %rm, implicit %x3
> > +
> > +...
> > +---
> > name: testRLWINM
> > # CHECK-ALL: name: testRLWINM
> > alignment: 4
> > @@ -4163,6 +4235,69 @@ body: |
> > %5 = COPY killed %cr0
> > %6 = ISEL %2, %3, %5.sub_eq
> > %8 = IMPLICIT_DEF
> > + %7 = INSERT_SUBREG %8, killed %6, 1
> > + %9 = RLDICL killed %7, 0, 32
> > + %x3 = COPY %9
> > + BLR8 implicit %lr8, implicit %rm, implicit %x3
> > +
> > +...
> > +---
> > +name: testRLWINMo2
> > +# CHECK-ALL: name: testRLWINMo2
> > +alignment: 4
> > +exposesReturnsTwice: false
> > +legalized: false
> > +regBankSelected: false
> > +selected: false
> > +tracksRegLiveness: true
> > +registers:
> > + - { id: 0, class: g8rc, preferred-register: '' }
> > + - { id: 1, class: g8rc, preferred-register: '' }
> > + - { id: 2, class: gprc_and_gprc_nor0, preferred-register: '' }
> > + - { id: 3, class: gprc_and_gprc_nor0, preferred-register: '' }
> > + - { id: 4, class: gprc, preferred-register: '' }
> > + - { id: 5, class: crrc, preferred-register: '' }
> > + - { id: 6, class: gprc, preferred-register: '' }
> > + - { id: 7, class: g8rc, preferred-register: '' }
> > + - { id: 8, class: g8rc, preferred-register: '' }
> > + - { id: 9, class: g8rc, preferred-register: '' }
> > +liveins:
> > + - { reg: '%x3', virtual-reg: '%0' }
> > + - { reg: '%x4', virtual-reg: '%1' }
> > +frameInfo:
> > + isFrameAddressTaken: false
> > + isReturnAddressTaken: false
> > + hasStackMap: false
> > + hasPatchPoint: false
> > + stackSize: 0
> > + offsetAdjustment: 0
> > + maxAlignment: 0
> > + adjustsStack: false
> > + hasCalls: false
> > + stackProtector: ''
> > + maxCallFrameSize: 4294967295
> > + hasOpaqueSPAdjustment: false
> > + hasVAStart: false
> > + hasMustTailInVarArgFunc: false
> > + savePoint: ''
> > + restorePoint: ''
> > +fixedStack:
> > +stack:
> > +constants:
> > +body: |
> > + bb.0.entry:
> > + liveins: %x3, %x4
> > +
> > + %1 = COPY %x4
> > + %0 = COPY %x3
> > + %2 = COPY %1.sub_32
> > + %3 = LI -22
> > + %4 = RLWINMo %3, 5, 24, 31, implicit-def %cr0
> > + ; CHECK-NOT: ANDI
> > + ; CHECK-LATE-NOT: andi.
> > + %5 = COPY killed %cr0
> > + %6 = ISEL %2, %3, %5.sub_eq
> > + %8 = IMPLICIT_DEF
> > %7 = INSERT_SUBREG %8, killed %6, 1
> > %9 = RLDICL killed %7, 0, 32
> > %x3 = COPY %9
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180112/dc09738a/attachment.html>
More information about the llvm-commits
mailing list