478158 - configure option for processor type (ARMv4, ARMv5, ARMv6) for windows ce

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Brad Lassey [:blassey] (use needinfo?)]

bug 477678 switched us to target ARMv6, however the emulators default to ARMv4 and can only support ARMv5 with command line flags

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Attachment: configure ARM processor type

Added configure option --with-arm-processor-type like this:

--with-arm-processor-type=ARM_PROCESSOR_TYPE
                          ARM processor type for windows ce
                              4: ARMv4
                              5: ARMv5
                              6: ARMv6

"6" is used by default.

And -CPU ARM1136 option for armasm is appended only if ARM_PROCESSOR type is 6
 because I have no idea what value is appropriate for ARMv4 and ARMv5.

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 362848 [details] [diff] [review]
configure ARM processor type

technically the flag should probably be --arm-processor-arch.  Given that, arm-wince-as can use the -arch flag. 

We may still want to target a specific cpu core such as arm1136, in which case we may need a second flag (--arm-processor-type?) to specify it.  

fwiw, 4T and 5T should also be valid archs.

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

Jeff, Vlad, do we care about having the assembler set to ARM1136 versus ARMv6.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

(In reply to comment #3)
> (From update of attachment 362848 [details] [diff] [review])

> We may still want to target a specific cpu core such as arm1136, in which case
> we may need a second flag (--arm-processor-type?) to specify it.  

What about "--arm-processor-core"?

Comment 6

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to comment #5) 
> What about "--arm-processor-core"?

core would work fine.  Its meaning might be more obvious too.

from armasm, the valid values would be:
 ARM7TM | ARM8 | StrongARM1 | ARM10 |ARM10T | ARM10200 | XSCALE | ARM1136

CC'ing ted to see if he has an opinion

Comment 7

[(not currently active) Ted Mielczarek]

I haven't got a clue what the best naming is for these ARM things, I'm fine with whatever you want to do. (Also I watch all Build Config bugs by default.)

Comment 8

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Nope, 1136 is fine, we really want to target a specific architecture version I think.  The problem is that it might not be enough, given that, for example, some architectures have VFP, but the OS doesn't have it enabled.

Jeff and I think we can just do runtime detection -- on CE using SEH and on linux by parsing /proc/cpuinfo.

Comment 9

[Hiroyuki Ikezoe (:hiro)]

Attachment: configure ARM processor type v2

Append --with-arm-processor-arch and --with-arm-processor-core option for configure.
Append -arch option for armmasm only if --with-arm-processor-code is not specified (because -CPU options overrides -arch option).

Comment 10

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 362989 [details] [diff] [review]
 configure ARM processor type v2

looks good to me, I'd like ted to look at the configure bits though

Comment 11

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 362989 [details] [diff] [review]
 configure ARM processor type v2

No, I'm serious, let's not do this -- there is no need to propagate configure options for something that needs to be detected at runtime anyway.

We need to place an arm features function somewhere; it's not hard to write, I just don't know where it should go.  nspr might be the most logical place (since we'll want to use it for linux as well, not just CE), but getting code in there is hard.  Any suggestions appreciated.

For armasm, we just want to always use arm1136; it just selects the instruction set that the assembler recognizes and limitations; anything we have in assembly should take all that into account already anyway.

The only thing that would need to be a flag is the compiler arch, and we shouldn't be specifying one explicitly anywhere -- anyone that wants something other than the default should just use CXXFLAGS="-QRarch6" or whatever's appropriate.

Comment 12

[Hiroyuki Ikezoe (:hiro)]

(In reply to comment #11)

> The only thing that would need to be a flag is the compiler arch, and we
> shouldn't be specifying one explicitly anywhere -- anyone that wants something
> other than the default should just use CXXFLAGS="-QRarch6" or whatever's
> appropriate.

Well, you mean removal /QRarch6 option from arm-wince-gcc.c is good to you?

Comment 13

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to comment #12)
> Well, you mean removal /QRarch6 option from arm-wince-gcc.c is good to you?

yes

Vlad, if we go this direction, should we just use ASFLAGS="arm1136" as well?

Comment 14

[Hiroyuki Ikezoe (:hiro)]

I(In reply to comment #13)
 
> Vlad, if we go this direction, should we just use ASFLAGS="arm1136" as well?

If so, I am happy. 
I am using Windows Mobile Emulator.

Comment 15

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #13)
> (In reply to comment #12)
> > Well, you mean removal /QRarch6 option from arm-wince-gcc.c is good to you?
> 
> yes
> 
> Vlad, if we go this direction, should we just use ASFLAGS="arm1136" as well?

Yeah, though for that we may as well hardcode it.. since it doesn't change the codegen at all, and code won't actually build without it (the assembler will refuse to assemble instructions that require a newer arch to execute, even if we have code to ensure that those functions will never get called if they're not supported).

Comment 16

[(not currently active) Ted Mielczarek]

Comment on attachment 362989 [details] [diff] [review]
 configure ARM processor type v2

Clearing this review request per vlad's comments. I'm fine with whatever vlad thinks is the right solution here. (Although do re-request review if your patch touches configure, please.)

Comment 17

[Hiroyuki Ikezoe (:hiro)]

The patch for Bug 479503 has already landed, this bug should be closed I think.
Brad, what do you think?

Comment 18

[Brad Lassey [:blassey] (use needinfo?)]

yup