Closed Bug 1480558 Opened 6 years ago Closed 6 years ago

figure out how to make ICU's data file work with aarch64 windows

Categories

(Firefox Build System :: General, enhancement)

ARM64
Windows
enhancement
Not set
normal

Tracking

(firefox63 fixed)

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

References

(Blocks 1 open bug)

Details

Attachments

(4 files, 1 obsolete file)

We generate a big data file for ICU, which we assume we can assembly with yasm or the GNU assembler.  Neither one is a viable option on aarch64 windows, and we *probably* can't disable ICU entirely.  So we need to find some kind of solution here.
Note that we cannot --without-intl-api our way to victory, because several places in the codebase depend on ICU's routines and have no fallback.
armasm64 might use INCBIN syntax to include binary.

DATA_SYMBOL
  INCBIN icudt62l.dat
  END
In the worst case if you can't find a solution that works with the available assembler, you could generate an equivalent C file that contains the contents of the .dat file as an array of bytes.
Attachment #8998615 - Flags: review?(core-build-config-reviews)
armasm64 doesn't accept the same options as its x86-ish counterparts,
and passing options it doesn't understand causes assembly to fail.  So
let's just not pass any flags to the assembler for the moment.
Attachment #8998616 - Flags: review?(core-build-config-reviews)
The assembler for this platform doesn't need the special handling
AS_DASH_C_FLAG provides.
Attachment #8998617 - Flags: review?(core-build-config-reviews)
yasm doesn't support aarch64, and trying to use GNU as with an MSVC
build seems like sadness waiting to happen.  Instead, we'll generate our
own assembly file that armasm64 will accept.
Attachment #8998618 - Flags: review?(core-build-config-reviews)
triaging, assigning to :froydnj since he attached patches
Assignee: nobody → nfroyd
Attachment #8998615 - Flags: review?(core-build-config-reviews) → review+
Attachment #8998616 - Flags: review?(core-build-config-reviews) → review+
Attachment #8998617 - Flags: review?(core-build-config-reviews) → review+
Comment on attachment 8998618 [details] [diff] [review]
part 4 - add icudata support for aarch64 windows

>+import buildconfig

Doesn't look like this gets used.
Attachment #8998618 - Flags: review?(core-build-config-reviews) → review+
I thought TARGET_CPU would be more likely to have aarch64 in it, since
target_cpu is set by old-configure/autoconf and we have such an old version of
autoconf.  However, TARGET_CPU apparently wasn't being set (I didn't notice on
my AArch64 build because I have AS set), and that caused problems for "real"
Windows builds.  Actually looking at the code says that old-configure is
perfectly willing to deal properly with aarch64-* targets, so we're good just
adding a case here.  Carrying over r+.
Attachment #9002751 - Flags: review+
Attachment #8998615 - Attachment is obsolete: true
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/edc9c6244815
part 1 - set AS appropriately on aarch64 windows; r=mshal
https://hg.mozilla.org/integration/mozilla-inbound/rev/7378a8999aea
part 2 - don't add MOZ_DEBUG_FLAGS to ASFLAGS on aarch64 windows; r=mshal
https://hg.mozilla.org/integration/mozilla-inbound/rev/90a93173d247
part 3 - unset AS_DASH_C_FLAG for aarch64 windows; r=mshal
https://hg.mozilla.org/integration/mozilla-inbound/rev/8eac225243d8
part 4 - add icudata support for aarch64 windows; r=mshal
You need to log in before you can comment on or make changes to this bug.