Bug 62916

Summary: WebCore fails to compile in thumb mode when neon instructions are enabled.
Product: WebKit Reporter: Raymes Khoury <raymes>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: maruel, tony, webkit.review.bot, zherczeg
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
Force -marm to be passed when building these files.
none
Force -marm to be passed when building these files.
none
Force -marm to be passed when building these files.
tony: review-
Force -marm to be passed when building these files.
none
Force -marm to be passed when building these files. none

Description Raymes Khoury 2011-06-17 16:31:03 PDT
When the -mthumb option is passed to GCC and neon instructions are enabled, FELightingNEON.cpp fails to be assembled with the following error:
{standard input}: Assembler messages:
{standard input}:56: Error: Thumb does not support negative register indexing -- `ldrb r0,[r4,-r12]'
{standard input}: Assembler messages:
{standard input}:177: Warning: conditional infixes are deprecated in unified syntax

make: *** [c/Release/obj.target/webcore_platform/third_party/WebKit/Source/WebCore/platform/graphics/filters/arm/FELightingNEON.o] Error 1
Comment 1 Raymes Khoury 2011-06-23 16:47:29 PDT
Similarly, FEGaussianBlurNEON.cpp fails to assemble with the following error:

{standard input}: Assembler messages:
{standard input}:39: Error: thumb conditional instruction should be in IT block -- `movcs lr,r7'
{standard input}:107: Error: thumb conditional instruction should be in IT block -- `movcs lr,r7'
{standard input}:177: Warning: conditional infixes are deprecated in unified syntax
{standard input}:177: Error: thumb conditional instruction should be in IT block -- `ldmeqia sp!,{r4-r8,r10,r11,pc}'
{standard input}:186: Error: thumb conditional instruction should be in IT block -- `vldrcs s9,[r2]'
{standard input}:189: Error: thumb conditional instruction should be in IT block -- `vldrcs s10,[r2]'
{standard input}:208: Error: thumb conditional instruction should be in IT block -- `vstrcs s9,[r1]'
{standard input}:211: Error: thumb conditional instruction should be in IT block -- `vstrcs s10,[r1]'
{standard input}:217: Error: thumb conditional instruction should be in IT block -- `vldrcs s9,[r2]'
{standard input}:220: Error: thumb conditional instruction should be in IT block -- `vldrcs s10,[r2]'
{standard input}:229: Error: thumb conditional instruction should be in IT block -- `vldrcs s9,[r3]'
{standard input}:232: Error: thumb conditional instruction should be in IT block -- `vldrcs s10,[r3]'


Everything else builds correctly.
Comment 2 Raymes Khoury 2011-06-23 17:20:52 PDT
Created attachment 98440 [details]
Force -marm to be passed when building these files.
Comment 3 Raymes Khoury 2011-06-23 17:31:15 PDT
Created attachment 98442 [details]
Force -marm to be passed when building these files.
Comment 4 WebKit Review Bot 2011-06-23 17:35:13 PDT
Attachment 98442 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:15:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 6 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Raymes Khoury 2011-06-23 17:42:41 PDT
Created attachment 98444 [details]
Force -marm to be passed when building these files.
Comment 6 Tony Chang 2011-06-27 14:35:34 PDT
Comment on attachment 98444 [details]
Force -marm to be passed when building these files. 

View in context: https://bugs.webkit.org/attachment.cgi?id=98444&action=review

Just some minor style nits.

> WebCore/ChangeLog:7
> +        enabled. There are two files (FELightingNEON.cpp and
> +        FELightingNEON.cpp) which fail to build when -mthumb is

I think you mean to mention FEGaussianBlur.cpp here.

> WebCore/WebCore.gyp/WebCore.gyp:1313
> +        ['exclude', 'platform/graphics/filters/arm/.*NEON\\.(cpp|h)'],

Nit: Sort the excludes alphabetically.  You might want to add a comment above this saying that these files are compiled into webcore_arm_neon.

> WebCore/WebCore.gyp/WebCore.gyp:1693
> +        'webcore_arm_neon',

Nit: Sort these alphabetically too.
Comment 7 Raymes Khoury 2011-06-27 15:10:29 PDT
Created attachment 98799 [details]
Force -marm to be passed when building these files.
Comment 8 WebKit Review Bot 2011-06-27 15:14:15 PDT
Attachment 98799 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/ChangeLog:6:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:7:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:8:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:9:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:10:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:15:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 6 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Raymes Khoury 2011-06-27 15:22:53 PDT
Created attachment 98800 [details]
Force -marm to be passed when building these files.
Comment 10 Tony Chang 2011-06-27 15:31:56 PDT
I'll land this once the ews bots go green.
Comment 11 WebKit Review Bot 2011-06-27 16:09:35 PDT
Comment on attachment 98800 [details]
Force -marm to be passed when building these files. 

Clearing flags on attachment: 98800

Committed r89874: <http://trac.webkit.org/changeset/89874>
Comment 12 WebKit Review Bot 2011-06-27 16:09:40 PDT
All reviewed patches have been landed.  Closing bug.