[llvm-commits] [PATCH] Atomic NAND options

[Cameron McInally]

Well... it is a change of semantics. Users who expect the intrinsic to
work the old way will be surprised when tests begin to fail. But, in
my opinion, there's no reason to keep the old implementation.

Perhaps a warning should be issued to inform the user that the
underlying implementation has changed. That's what GCC has done to
address this issue.

On Thursday, July 14, 2011, Chris Lattner <clattner at apple.com> wrote:
>
> On Jul 14, 2011, at 8:03 AM, Cameron McInally wrote:
> Hey Guys,
>
> I have been working on Atomic NAND. Here is a patch that allows the
> compiler writer to select a [NOT AND] implementation, while retaining
> [NEGATE and AND] as the default implementation for those that do not
> want the current GCC implementation (i.e. GCC v4.4 and later).
>
> Hi Cameron,
> Is there a reason to support the broken pre-gcc-4.4 implementation at all?
> -Chris
>
> upstream/llvm> svn diff
> Index: lib/Target/X86/X86ISelLowering.cpp
> ===================================================================
> --- lib/Target/X86/X86ISelLowering.cpp    (revision 135054)
> +++ lib/Target/X86/X86ISelLowering.cpp    (working copy)
> @@ -9710,7 +9710,8 @@
>                                                         unsigned notOpc,
>                                                         unsigned EAXreg,
>
> TargetRegisterClass *RC,
> -                                                       bool invSrc) const {
> +                                                       bool invSrc,
> +                                bool invRes) const {
>    // For the atomic bitwise operator, we generate
>    //   thisMBB:
>    //   newMBB:
> @@ -9783,13 +9784,20 @@
>    MIB.addReg(tt);
>    (*MIB).addOperand(*argOpers[valArgIndx]);
>
> +  unsigned tr = F->getRegInfo().createVirtualRegister(RC);
> +  if (invRes) {
> +    MIB = BuildMI(newMBB, dl, TII->get(notOpc), tr).addReg(t2);
> +  }
> +  else
> +    tr = t2;
> +
>    MIB = BuildMI(newMBB, dl, TII->get(TargetOpcode::COPY), EAXreg);
>    MIB.addReg(t1);
>
>    MIB = BuildMI(newMBB, dl, TII->get(CXchgOpc));
>    for (int i=0; i <= lastAddrIndx; ++i)
>      (*MIB).addOperand(*argOpers[i]);
> -  MIB.addReg(t2);
> +  MIB.addReg(tr);
>    assert(bInstr->hasOneMemOperand() && "Unexpected number of memoperand");
>    (*MIB).setMemRefs(bInstr->memoperands_begin(),
>                      bInstr->memoperands_end());
> @@ -9812,7 +9820,8 @@
>                                                         unsigned regOpcH,
>                                                         unsigned immOpcL,
>                                                         unsigned immOpcH,
> -                                                       bool invSrc) const {
> +                                                       bool invSrc,
> +                                bool invRes) const {
>    // For the atomic bitwise operator, we generate
>    //   thisMBB (instructions are in pairs, except cmpxchg8b)
>    //     ld t1,t2 = [bitinstr.addr]
> @@ -9939,15 +9948,26 @@
>      MIB.addReg(t2);
>    (*MIB).addOperand(*argOpers[valArgIndx + 1]);
>
> +  unsigned trl = F->getRegInfo().createVirtualRegister(RC);
> +  unsigned trh = F->getRegInfo().createVirtualRegister(RC);
> +  if (invRes) {
> +    MIB = BuildMI(newMBB, dl, TII->get(NotOpc), trl).addReg(t5);
> +    MIB = BuildMI(newMBB, dl, TII->get(NotOpc), trh).addReg(t6);
> +  }
> +  else {
> +    trl = t5;
> +    trh = t6;
> +  }
> +
>    MIB = BuildMI(newMBB, dl, TII->get(TargetOpcode::COPY), X86::EAX);
>    MIB.addReg(t1);
>    MIB = BuildMI(newMBB, dl, TII->get(TargetOpcode::COPY), X86::EDX);
>    MIB.addReg(t2);
>
>    MIB = BuildMI(newMBB, dl, TII->get(TargetOpcode::COPY), X86::EBX);
> -  MIB.addReg(t5);
> +  MIB.addReg(trl);
>    MIB = BuildMI(newMBB, dl, TII->get(TargetOpcode::COPY), X86::ECX);
> -  MIB.addReg(t6);
> +  MIB.addReg(trh);
>
>    MIB = BuildMI(newMBB, dl, TII->get(X86::LCMPXCHG8B));
>    for (int i=0; i <= lastAddrIndx; ++i)
> Index: lib/Target/X86/X86ISelLowering.h
> ===================================================================
> --- lib/Target/X86/X86ISelLowering.h    (revision 135054)
> +++ lib/Target/X86/X86ISelLowering.h    (working copy)
> @@ -896,7 +896,8 @@
>                                                      unsigned notOpc,
>                                                      unsigned EAXreg,
>                                                      TargetRegisterClass
> *RC,
> -                                                    bool invSrc =
> false) const;
> +                                                    bool invSrc = false,
> +                             bool invRes = false) const;
>
>      MachineBasicBlock *EmitAtomicBit6432WithCustomInserter(
>                                                      MachineInstr *BInstr,
> @@ -905,7 +906,8 @@
>                                                      unsigned regOpcH,
>                                                      unsigned immOpcL,
>                                                      unsigned immOpcH,
> -                                                    bool invSrc =
> false) const;
> +                                                    bool invSrc = false,
> +                             bool invRes = false) const;
>
>      /// Utility function to emit atomic min and max.  It takes the min/max
>      /// instruction to expand, the associated basic block, and the
> associated
>
>
> With the above patch, the current LLVM implementation will be selected.
> To enablCameron
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>