On 03.02.2012 02:48, Bruce Momjian wrote:
Sorry for the late reply, but Heikki, can you get this Itanium
information into s_lock.h as a comment, particularly the information
about the +Ovolatile=__unordered flag?
Good idea. I came up with the attached, hope that explains it.
Looking back at the discussions, we concluded that the current code is
safe on gcc, because it implicitly adds the .rel/.acq opcodes to
volatile accesses, and HP's compiler does the same as long as you don't
explicitly disable it with +Ovolatile=__unordered. But what about
Intel's icc compiler? Presumably it's also safe, but looking at Intel's
manuals that I found, I'm not completely sure about it. There's an
option, -m[no-]serialize-volatile, that controls it, but I couldn't
figure out which is the default. Looking at the docs on that from Intel
that I found [1], it seems to me that on Linux, the default is *not*
safe, but on Windows it is.
Sergey, you have dugong in the buildfarm that uses Intel's compiler on
Itanium. Could you verify whether the -mno-serialize-volatile is the
default? If you could for example extract the assembler code generated
by icc for xlog.c, and send it over. Whether it's generating the
.rel/.acq opcodes should be easy to see in the generated code of the
XLogGetLastRemoved() function, for example, which doesn't do much else
than grab a spinlock. On gcc, the -s flag generates the assembly files,
I presume it's the same on icc.
Perhaps we should set those compiler flags explicitly in configure,
regardless of the defaults.
[1]
http://software.intel.com/sites/products/documentation/hpc/compilerpro/en-us/cpp/lin/compiler_c/copts/ccpp_options/option_qserialize-volatile.htm
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
index 074838e..d55b662 100644
--- a/src/include/storage/s_lock.h
+++ b/src/include/storage/s_lock.h
@@ -234,7 +234,22 @@ spin_delay(void)
#endif /* __x86_64__ */
-#if defined(__ia64__) || defined(__ia64) /* Intel Itanium */
+#if defined(__ia64__) || defined(__ia64)
+/*
+ * Intel Itanium, gcc or Intel's compiler.
+ *
+ * Itanium has weak memory ordering, but we rely on the compiler to enforce
+ * strict ordering of accesses to volatile data. In particular, while the
+ * xchg instruction implicitly acts as a memory barrier with 'acquire'
+ * semantics, we do not have an explicit memory fence instruction in the
+ * S_UNLOCK macro. We use a regular assignment to clear the spinlock, and
+ * trust that the compiler marks the generated store instruction with the
+ * ".rel" opcode.
+ *
+ * Testing shows that assumption holds on gcc, although I could not find any
+ * official statement on that in the gcc manual. In Intel's compiler, the
+ * -m[no-]serialize-volatile option controls that. Keep it enabled!
+ */
#define HAS_TEST_AND_SET
typedef unsigned int slock_t;
@@ -785,7 +800,19 @@ tas(volatile slock_t *lock)
#if defined(__hpux) && defined(__ia64) && !defined(__GNUC__)
-
+/*
+ * HP-UX on Itanium, non-gcc compiler
+ *
+ * We assume that the compiler enforces strict ordering of loads/stores on
+ * volatile data (see comments on the gcc-version earlier in this file).
+ * Note that this assumption does *not* hold if you use the
+ * +Ovolatile=__unordered option on the HP-UX compiler, so don't do that.
+ *
+ * See also Implementing Spinlocks on the Intel Itanium Architecture and
+ * PA-RISC, by Tor Ekqvist and David Graves, for more information. As of
+ * this writing, version 1.0 of the manual is available at:
+ * http://h21007.www2.hp.com/portal/download/files/unprot/itanium/spinlocks.pdf
+ */
#define HAS_TEST_AND_SET
typedef unsigned int slock_t;
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
