Bug 126985

Summary: Source/WTF/wtf/Atomics.h:300: Error: bad register name `%bpl'
Product: WebKit Reporter: Alberto Garcia <berto>
Component: Web Template FrameworkAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cmarcelo, commit-queue, David.Ronis, oliver, tpopela, zan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch ossy: review+

Description Alberto Garcia 2014-01-14 07:39:38 PST
I'm having problems building the GTK+ port in i386. Looks like gcc is producing x86-64 registers here:

inline bool weakCompareAndSwap(uint8_t* location, uint8_t expected, uint8_t newValue)
{
    unsigned char result;
    asm volatile(
        "lock; cmpxchgb %3, %2\n\t"
        "sete %1"
        : "+a"(expected), "=q"(result), "+m"(*location)
        : "r"(newValue)
        : "memory"
        );
    return result;
}

Here's the gcc output of the affected pre-processed source file:

$ gcc -O1 -m32 --std=c++11 -c CodeBlock.ii 
../Source/WTF/wtf/Atomics.h: Assembler messages:
../Source/WTF/wtf/Atomics.h:300: Error: bad register name `%bpl'
../Source/WTF/wtf/Atomics.h:300: Error: bad register name `%bpl'
../Source/WTF/wtf/Atomics.h:300: Error: bad register name `%dil'

Adding -frename-registers seems to solve the problem. It also builds fine with clang.

My understanding is that this is a gcc bug, however reading the upstream
report http://gcc.gnu.org/bugzilla/show_bug.cgi?id=10153 and its duplicates
suggests that the constraints in the function I pasted above might be wrong.
Comment 1 Zan Dobersek 2014-01-14 10:26:07 PST
*** Bug 126989 has been marked as a duplicate of this bug. ***
Comment 2 David Ronis 2014-01-14 14:35:21 PST
I added -frename-registers to the build CFLAGS and CXXFLAGS.  I get the exact same message as in my initial report:

./Source/WTF/wtf/Atomics.h: Assembler messages:
./Source/WTF/wtf/Atomics.h:300: Error: bad register name `%sil'
Comment 3 Alberto Garcia 2014-01-16 04:39:04 PST
Created attachment 221366 [details]
Patch

The problem is that with the "r" constraint we are telling gcc to put
'newValue' into any general register, which is then used as an operand
for cmpxchgb.

However in i386 not all registers allow access to their lower byte,
which is what we need here.

The "q" constraint ensures that in 32-bit mode only the a, b, c and d
registers are used.
Comment 4 WebKit Commit Bot 2014-01-16 04:40:37 PST
Attachment 221366 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/Atomics.h', '--commit-queue']" exit_code: 1
ERROR: Source/WTF/wtf/Atomics.h:298:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/WTF/wtf/Atomics.h:298:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Csaba Osztrogon√°c 2014-01-16 08:47:51 PST
Comment on attachment 221366 [details]
Patch

LGTM, r=me
Comment 6 Alberto Garcia 2014-01-16 11:19:42 PST
Committed r162137: <http://trac.webkit.org/changeset/162137>
Comment 7 David Ronis 2014-01-16 13:39:43 PST
I just finished my build with the patch, it worked (Note:  I still had 
-frename-registers in my CFLAGS/CXXFLAGS)

Thanks