Bug 130500

Summary: [ARM64] GCC generates wrong code with -O2 flag in WTF::weakCompareAndSwap
Product: WebKit Reporter: Gabor Rapcsanyi <rgabor>
Component: Web Template FrameworkAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cmarcelo, commit-queue, fpizlo, oliver, ossy, zherczeg
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Linux   
Bug Depends on:    
Bug Blocks: 108645, 130506    
Attachments:
Description Flags
proposed fix
none
proposed patch v2 none

Gabor Rapcsanyi
Reported 2014-03-20 02:28:12 PDT
/tmp/cc2M6EGp.s:23: Error: the optional immediate offset can only be 0 at operand 2 -- `ldxr w3,[x29,28]' /tmp/cc2M6EGp.s:26: Error: the optional immediate offset can only be 0 at operand 3 -- `stxr w2,w1,[x29,28]'
Attachments
proposed fix (1.85 KB, patch)
2014-03-20 02:38 PDT, Gabor Rapcsanyi 		no flags
DetailsFormatted DiffDiff
proposed patch v2 (1.94 KB, patch)
2014-03-25 02:50 PDT, Gabor Rapcsanyi 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Gabor Rapcsanyi
Comment 1 2014-03-20 02:37:55 PDT
Tried with: gcc-linaro 4.8 (Aarch64) 4.8-2014.02 14.02 (http://www.linaro.org/downloads/)
Gabor Rapcsanyi
Comment 2 2014-03-20 02:38:35 PDT
Created attachment 227276 [details] proposed fix
WebKit Commit Bot
Comment 3 2014-03-20 02:40:21 PDT
Attachment 227276 [details] did not pass style-queue: ERROR: Source/WTF/wtf/Atomics.h:125: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:125: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:125: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:125: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:125: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:125: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:165: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:165: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:165: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:165: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] Total errors found: 10 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Zoltan Herczeg
Comment 4 2014-03-24 01:38:52 PDT
This refactor is also ok. I think all of these patches can be landed, if nobody has an objection.
Filip Pizlo
Comment 5 2014-03-24 07:52:05 PDT
Comment on attachment 227276 [details] proposed fix Since the new constraints are incorrect and are just there to appease gcc, I would put them behind some #if's and keep the original code around. My recollection is that not using +m in a cas can also lead to code gen bugs in some compilers because the "memory" constraint isn't enough to make them see what's going on.
Gabor Rapcsanyi
Comment 6 2014-03-25 02:50:24 PDT
Created attachment 227731 [details] proposed patch v2 I put the changes between GCC guards.
WebKit Commit Bot
Comment 7 2014-03-25 02:53:22 PDT
Attachment 227731 [details] did not pass style-queue: ERROR: Source/WTF/wtf/Atomics.h:125: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:126: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:125: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:126: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:125: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:126: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:125: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:126: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:125: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:126: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:125: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:126: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:126: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:179: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:180: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:179: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:180: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:179: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:180: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:179: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:180: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] ERROR: Source/WTF/wtf/Atomics.h:180: Wrong number of spaces before statement. (expected: 12) [whitespace/indent] [4] Total errors found: 22 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
WebKit Commit Bot
Comment 8 2014-03-25 07:13:34 PDT
Comment on attachment 227731 [details] proposed patch v2 Clearing flags on attachment: 227731 Committed r166234: <http://trac.webkit.org/changeset/166234>
WebKit Commit Bot
Comment 9 2014-03-25 07:13:38 PDT
All reviewed patches have been landed. Closing bug.
Andreas Schwab
Comment 10 2015-06-15 06:08:23 PDT
The correct constraint is "Q".
Note You need to log in before you can comment on or make changes to this bug.