[llvm] r212443 - CodeGen: it turns out that NAND is not the same thing as BIC. At all.

[Cameron McInally]

On Tue, Jul 8, 2014 at 1:08 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> ----- Original Message -----
>> From: "Cameron McInally" <cameron.mcinally at nyu.edu>
>> To: "Hal Finkel" <hfinkel at anl.gov>
>> Cc: "Duncan Sands" <duncan.sands at gmail.com>, llvm-commits at cs.uiuc.edu, "Tim Northover" <t.p.northover at gmail.com>
>> Sent: Tuesday, July 8, 2014 11:58:56 AM
>> Subject: Re: [llvm] r212443 - CodeGen: it turns out that NAND is not the same thing as BIC. At all.
>>
>> On Tue, Jul 8, 2014 at 12:31 PM, Hal Finkel <hfinkel at anl.gov> wrote:
>> > ----- Original Message -----
>> >> From: "Cameron McInally" <cameron.mcinally at nyu.edu>
>> >> To: "Hal Finkel" <hfinkel at anl.gov>
>> >> Cc: "Duncan Sands" <duncan.sands at gmail.com>,
>> >> llvm-commits at cs.uiuc.edu, "Tim Northover"
>> >> <t.p.northover at gmail.com>
>> >> Sent: Monday, July 7, 2014 8:58:24 AM
>> >> Subject: Re: [llvm] r212443 - CodeGen: it turns out that NAND is
>> >> not the same thing as BIC. At all.
>> >>
>> >> On Mon, Jul 7, 2014 at 9:44 AM, Hal Finkel <hfinkel at anl.gov>
>> >> wrote:
>> >> > ----- Original Message -----
>> >> >> From: "Duncan Sands" <duncan.sands at gmail.com>
>> >> >> To: llvm-commits at cs.uiuc.edu
>> >> >> Sent: Monday, July 7, 2014 4:35:37 AM
>> >> >> Subject: Re: [llvm] r212443 - CodeGen: it turns out that NAND
>> >> >> is
>> >> >> not the same thing as BIC. At all.
>> >> >>
>> >> >> Hi Tim,
>> >> >>
>> >> >> On 07/07/14 11:06, Tim Northover wrote:
>> >> >> > Author: tnorthover
>> >> >> > Date: Mon Jul  7 04:06:35 2014
>> >> >> > New Revision: 212443
>> >> >> >
>> >> >> > URL: http://llvm.org/viewvc/llvm-project?rev=212443&view=rev
>> >> >> > Log:
>> >> >> > CodeGen: it turns out that NAND is not the same thing as BIC.
>> >> >> > At
>> >> >> > all.
>> >> >> >
>> >> >> > We've been performing the wrong operation on ARM for
>> >> >> > "atomicrmw
>> >> >> > nand" for
>> >> >> > years, since "a NAND b" is "~(a & b)" rather than ARM's very
>> >> >> > tempting "a & ~b".
>> >> >> > This bled over into the generic expansion pass.
>> >> >> >
>> >> >> > So I assume no-one has ever actually tried to do an atomic
>> >> >> > nand
>> >> >> > in
>> >> >> > the real
>> >> >> > world. Oh well.
>> >> >>
>> >> >> this may have been a feature, not a mistake: it might simply
>> >> >> have
>> >> >> been trying to
>> >> >> be compatible with GCC which also did a & ~b for atomic nand
>> >> >> for
>> >> >> years (it was
>> >> >> even documented to be this way).  The semantics were changed to
>> >> >> ~(a &
>> >> >> b) in GCC
>> >> >> 4.4.  See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=37908
>> >> >
>> >> > Is this a problem, then, because we claim compatibility with gcc
>> >> > 4.2.1? I wonder if this breaks anything in the wild.
>> >>
>> >> If it helps... I intended to change this in 2011, but dropped the
>> >> ball
>> >> on non-X86 backends.
>> >>
>> >> https://groups.google.com/forum/#!topic/llvm-commit/RLFo4DHaYR8
>> >
>> > Okay, so ARM and PPC have now been updated. Does X86 still need to
>> > be updated? If so, we better do that now ;)
>>
>> Hey Hal,
>>
>> The x86 atomics were rewritten shortly after the 2011 conversation,
>> where the NAND operation was updated to ~(a&b).
>>
>> I just double-checked the x86 backend and the source matches my
>> memory.
>>
>> We also have tests to check for the correct assembly sequence. E.g.
>>
>> ; X64:       andb
>> ; X64:       notb
>> ; X64:       lock
>> ; X64:       cmpxchgb
>
> Okay, great.
>
>>
>>
>> Also, if you would like help with the documentation or WARNING, I can
>> assist you with this work.
>
> Given that the X86 backend has been updated for several years, I'm not sure the warning is relevant now. However, we should certainly add a note to the documentation re: the change and the release notes. If you can take care of this, that would be great!
>

Hal,

I do not often modify the docs, so please feel free to critique harshly. ;)

A small patch is attached.

-Cameron
-------------- next part --------------
Index: ReleaseNotes.rst
===================================================================
--- ReleaseNotes.rst	(revision 212617)
+++ ReleaseNotes.rst	(working copy)
@@ -59,6 +59,9 @@
 * The prefix for loop vectorizer hint metadata has been changed from
   ``llvm.vectorizer`` to ``llvm.loop.vectorize``.
 
+* Some backends previously implemented Atomic NAND as NEGATE-and-AND. The
+  semantics of this operation have been updated to NOT-AND.
+
 .. NOTE
    For small 1-3 sentence descriptions, just add an entry at the end of
    this list. If your description won't fit comfortably in one bullet