Thread: Build failure on m68k and ia64: inconsistent operand constraints in an `asm'

Hi PostgreSQL developers!

Recently PostgreSQL did not build any more on the Debian ia64 and m68k buildds:

----------
m68k-linux-gcc -DCHECK_RLIMIT_NOFILE -fno-strict-aliasing -g -Wall -Wmissing-prototypes -Wmissing-declarations -pipe
-I../../../../src/include-D_GNU_SOURCE  -I/usr/include/tcl8.4  -c -o xlog.o xlog.c 
../../../../src/include/storage/s_lock.h: In function `tas':
../../../../src/include/storage/s_lock.h:292: error: inconsistent operand constraints in an `asm'
make[5]: *** [xlog.o] Error 1
----------

The relevant code on m68K:

----------
        __asm__ __volatile__(
                "       clrl    %0              \n"
                "       tas             %1              \n"
                "       sne             %0              \n"
:               "=d"(rv), "=m"(*lock)
:               "1"(*lock)
:               "cc");
----------

and on IA64:

----------
        __asm__ __volatile__(
                "       xchg4   %0=%1,%2        \n"
:               "=r"(ret), "=m"(*lock)
:               "r"(1), "1"(*lock)
:               "memory");
----------

A Debian porter suggested that "1"(*lock) is an obsolete syntax and
should be replaced by "m"(*lock) in both cases; however, I would like
to get a second opinion about this.

Does this make sense? Would you consider doing this in the next
release?

Thanks and have a nice day!

Martin

--
Martin Pitt                 Debian GNU/Linux Developer
martin@piware.de                      mpitt@debian.org
http://www.piware.de             http://www.debian.org

Attachment
Martin Pitt <martin@piware.de> writes:
> Recently PostgreSQL did not build any more on the Debian ia64 and m68k buil=
> dds:

Just to clarify --- what you're reporting is that Debian changed their
compiler to break our code, right?  Because this asm has been the same
for quite awhile ...

> A Debian porter suggested that "1"(*lock) is an obsolete syntax and
> should be replaced by "m"(*lock) in both cases; however, I would like
> to get a second opinion about this.

We will need to find out whether this syntax also works with older
gccs, and if not, what hoops we must leap through to determine which
syntax to use.

            regards, tom lane

Hi Tom!

On 2004-06-10  0:37 -0400, Tom Lane wrote:
> Martin Pitt <martin@piware.de> writes:
> > Recently PostgreSQL did not build any more on the Debian ia64 and m68k buil=
> > dds:
>
> Just to clarify --- what you're reporting is that Debian changed their
> compiler to break our code, right?  Because this asm has been the same
> for quite awhile ...

I know, but it would be nice to be able to compile PostgreSQL with
recent compilers. This was not meant as a bug report, just a question
to people who know more about assembler than me. Our porters proposed
the new solution with "m"(*lock), but I wanted to get some more
opinions.

> > A Debian porter suggested that "1"(*lock) is an obsolete syntax and
> > should be replaced by "m"(*lock) in both cases; however, I would like
> > to get a second opinion about this.
>
> We will need to find out whether this syntax also works with older
> gccs, and if not, what hoops we must leap through to determine which
> syntax to use.

Concerning Debian it is not really required that it works also with
old compilers (well, 2.95 would be nice, but it is not necessary). It
is perfectly okay for me to patch this only for the Debian version.

So does this statement make sense?

Thanks in advance!

Martin

--
Martin Pitt                 Debian GNU/Linux Developer
martin@piware.de                      mpitt@debian.org
http://www.piware.de             http://www.debian.org

Attachment

Re: Build failure on m68k and ia64: inconsistent operand constraints in an `asm'

From
Peter Eisentraut
Date:
Martin Pitt wrote:
> A Debian porter suggested that "1"(*lock) is an obsolete syntax and
> should be replaced by "m"(*lock) in both cases; however, I would like
> to get a second opinion about this.

If it were obsolete syntax, then it would still work.  As it is, they
are treating it as invalid syntax, which is really a bad move on their
part.


Hi again!

On 2004-06-10 12:04 +0200, Peter Eisentraut wrote:
> Martin Pitt wrote:
> > A Debian porter suggested that "1"(*lock) is an obsolete syntax and
> > should be replaced by "m"(*lock) in both cases; however, I would like
> > to get a second opinion about this.
>
> If it were obsolete syntax, then it would still work.

Sorry, I expressed myself incorrectly: according to the porters, this
is obsolete already for a long time and became invalid just recently.

> As it is, they are treating it as invalid syntax, which is really a
> bad move on their part.

Maybe, but at some time the gcc guys just have to get rid of old
syntax, they should not keep it forever. Anyway, somehow I have to
deal with this situation. Of course I can just do the patch, upload it
and see whether it works, but I would like it much more to get an
opinion "yes, this makes sense" or "no, this means something entirely
different" before.

Thanks,

Martin

--
Martin Pitt                 Debian GNU/Linux Developer
martin@piware.de                      mpitt@debian.org
http://www.piware.de             http://www.debian.org

Attachment
Martin Pitt <martin@piware.de> writes:
> A Debian porter suggested that "1"(*lock) is an obsolete syntax and
> should be replaced by "m"(*lock) in both cases; however, I would like
> to get a second opinion about this.

Having now re-read the gcc asm info, I think the above suggestion is
completely wrong.  I looked at gcc 2.95, 3.2, and 3.3.4 texinfo
documentation (the versions I have handy) and they all say exactly
the same thing:

: Only a number in the constraint can guarantee that one operand will
: be in the same place as another.  The mere fact that `foo' is the value
: of both operands is not enough to guarantee that they will be in the
: same place in the generated assembler code.

There is no hint that using a number is deprecated or might go away
in the future.

There is a mention that "+" is an alternative syntax for specifying
read-write operands, but considering that none of our ports have
ever used this, I do not know what sorts of problems we might be in for
if we switch over to that approach.  We will definitely risk breakage
if we don't have any constraint that the spinlock input and output values
are the same operand.

I think Debian broke their compiler and they ought to un-break it.

            regards, tom lane

Hi again!

On 2004-06-10 15:15 -0400, Tom Lane wrote:
> I think Debian broke their compiler and they ought to un-break it.

I asked Roland again, and got:

On 2004-06-11 10:10 +0200, Roman Zippel wrote:
> gcc developers disagree, we had same problem with the kernel, e.g. see:
>
> http://marc.theaimsgroup.com/?l=linux-kernel&m=107475162200773&w=2
>
> There is nothing broken with the compiler and changing the constraints of
> the memory operand to a "=m"/"m" pair is correctly understood by all
> versions of gcc.

Hmm. Since I cannot judge anyway, maybe you two can discuss that
directly and I implement whatever consensus you will reach?

Thanks and have a nice Sunday!

Martin

--
Martin Pitt                 Debian GNU/Linux Developer
martin@piware.de                      mpitt@debian.org
http://www.piware.de             http://www.debian.org

Attachment
Martin Pitt <martin@piware.de> writes:
> On 2004-06-11 10:10 +0200, Roman Zippel wrote:
>> gcc developers disagree, we had same problem with the kernel, e.g. see:
>>
>> http://marc.theaimsgroup.com/?l=3Dlinux-kernel&m=3D107475162200773&w=3D2

Yeah, and notice who he's arguing with ;-).

I'm with Linus on this one.  Every version of the gcc asm documentation
that I've looked at says I *must* use a match constraint to ensure that
input and output operands are in the same location.  I'm not inclined to
take one person's claim to the contrary as authoritative, especially not
when his argument is based on a statement about lvalues that isn't in
the docs at all.

BTW, if I read this message correctly, it's also saying that "+m"(*lock)
wouldn't work, which moves it out of the realm of reasonability
altogether.  Why in the world would an asm facility not support the
concept of a read-write operand in memory?

I was about to propose switching over to "+m" on the basis of some
advice I'd gotten internally at Red Hat, but now I think I will sit and
wait until some gcc developer shows me updated documentation that
actually explains what they think asm code should look like.

            regards, tom lane

Re: Build failure on m68k and ia64: inconsistent operand

From
Roman Zippel
Date:
Hi,

On Sun, 13 Jun 2004, Tom Lane wrote:

> Martin Pitt <martin@piware.de> writes:
> > On 2004-06-11 10:10 +0200, Roman Zippel wrote:
> >> gcc developers disagree, we had same problem with the kernel, e.g. see:
> >>
> >> http://marc.theaimsgroup.com/?l=3Dlinux-kernel&m=3D107475162200773&w=3D2
>
> Yeah, and notice who he's arguing with ;-).

But even Linus can't change the facts (if you follow the discussion,
you'll notice that he doesn't even try), that gcc cannot guarantee that
one memory expression is identical with another one without a reload into
a different location what would break the atomic behaviour which is needed
by many asm inline instructions (which is btw not documented either and
without it the asm at hand wouldn't work at all), so newer gcc releases
don't even pretend to do this anymore.

> I'm with Linus on this one.  Every version of the gcc asm documentation
> that I've looked at says I *must* use a match constraint to ensure that
> input and output operands are in the same location.  I'm not inclined to
> take one person's claim to the contrary as authoritative, especially not
> when his argument is based on a statement about lvalues that isn't in
> the docs at all.

He is a leading gcc developer and Linus does take his "claim" as
authoritative. Please take this to the gcc developers, but don't leave
certain machines broken.

> BTW, if I read this message correctly, it's also saying that "+m"(*lock)
> wouldn't work, which moves it out of the realm of reasonability
> altogether.  Why in the world would an asm facility not support the
> concept of a read-write operand in memory?
>
> I was about to propose switching over to "+m" on the basis of some
> advice I'd gotten internally at Red Hat, but now I think I will sit and
> wait until some gcc developer shows me updated documentation that
> actually explains what they think asm code should look like.

"+m" does work too, but gcc 3.4.0 will produce a warning for this (it's
fixed in 3.4.1, that simply will internally produce "=m"/"m" for this)
and it's doesn't work with gcc older than 2.95. (I don't know the compiler
requirements of postgresql).
Where is the problem with changing it simply into "=m"/"m"? It is a
correct solution, if something is wrong with it, so would it be for "+m"
or "=m"/"0", so I'm a bit puzzled what you're trying to discuss here...

bye, Roman