|From:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>|
|To:||David CARLIER <devnexen(at)gmail(dot)com>|
|Subject:||Re: [PATCH] Porting small OpenBSD changes.|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
David CARLIER <devnexen(at)gmail(dot)com> writes:
> On 20 November 2017 at 18:49, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> OTOH, we still have M68K
>> and VAX stanzas in that file, so I suppose it's silly to complain
>> about 88K. A bigger issue is that I wonder whether that code has
>> ever been tested: it does not look to me like the __asm__ call is
>> even syntactically correct. There should be colons in it.
> True :-) corrected. Thanks.
I still dare to doubt whether you've tested this, because AFAICS
the operand numbering is wrong. The "r"(lock) operand is number 3
given these operand declarations, not number 2.
Our practice elsewhere in s_lock.h is to use a "+" constraint instead of
duplicated operands, and I think that's better style because it avoids any
question of whether you're supposed to count duplicated operands.
Also, per the comment near s_lock.h line 115, it's important to specify
"+m"(*lock) as an output operand so that gcc knows that the asm
clobbers *lock. It's possible that "memory" makes that redundant,
but I'd just as soon maintain consistency with the well-tested
other parts of the file.
So I propose the attached patch instead. It would still be a good idea
to actually test this ;-)
regards, tom lane
|Next Message||Tom Lane||2017-11-20 22:36:15||Re: [PATCH] Porting small OpenBSD changes.|
|Previous Message||Masahiko Sawada||2017-11-20 22:19:30||Re: [HACKERS] Moving relation extension locks out of heavyweight lock manager|