| Summary: | [ARM64] GCC generates wrong code with -O2 flag in WTF::weakCompareAndSwap | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Gabor Rapcsanyi <rgabor> | ||||||
| Component: | Web Template Framework | Assignee: | 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
Gabor Rapcsanyi
2014-03-20 02:28:12 PDT
Tried with: gcc-linaro 4.8 (Aarch64) 4.8-2014.02 14.02 (http://www.linaro.org/downloads/) Created attachment 227276 [details]
proposed fix
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.
This refactor is also ok. I think all of these patches can be landed, if nobody has an objection. 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.
Created attachment 227731 [details]
proposed patch v2
I put the changes between GCC guards.
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.
Comment on attachment 227731 [details] proposed patch v2 Clearing flags on attachment: 227731 Committed r166234: <http://trac.webkit.org/changeset/166234> All reviewed patches have been landed. Closing bug. The correct constraint is "Q". |