Closed Bug 1723930 Opened 3 years ago Closed 3 years ago

Clean up unaligned load/store on ARM

Categories

(Core :: JavaScript: WebAssembly, enhancement, P2)

ARM
All
enhancement

Tracking

()

RESOLVED FIXED
92 Branch
Tracking Status
firefox-esr91 --- fixed
firefox92 --- fixed

People

(Reporter: lth, Assigned: lth)

References

Details

Attachments

(1 file)

Our codegen for unaligned load/store on ARM is unnecessarily complex. We are currently generating code to do individual byte loads and stores to assemble words when the alignment modifier on the access says that the access may not have natural alignment. But in practice, we are completely dependent on integer word loads and stores working even when the access is not aligned - we have no trap handling or other fallback for this (unlike for FP, though that's a complicated story, see bug 1587757). We should simplify this by just making that assumption explicit. We can subsequently fix bug 1587757 more easily.

Since early on, code generation on ARM has emulated accesses that are
declared as unaligned (the align= attribute has a value that is less
than the natural alignment for the datum) by emitting individual byte
accesses. There's almost no good reason to continue to do this, as we
are completely dependent on other mechanisms for handling accesses
that are actually unaligned (the effective address does not have
natural alignment) though declared to be aligned:

  • For integer accesses we are dependent on the hardware/OS to support
    actually-unaligned accesses transparently.

  • For FP accesses we handle signals originating from
    actually-unaligned accesses that are not handled by hardware/OS, but
    that code is being rewritten over in bug 1587757 to avoid the trap
    handling altogether by using unaligned-friendly instructions.

(Some of the code ripped out in this patch comes back in the patch for
bug 1587757, though in a different form.)

The only reason to continue using individual byte accesses would be
performance: if the CPU does not support unaligned accesses and the OS
has to emulate them in a trap handler, programs could be faster by
avoiding the traps. Yet such hardware is increasingly nonexistent.
Also, this solution would depend on the tools to declare accesses as
potentially-unaligned in all appropriate cases, and that is almost
certainly not going to happen.

Pushed by lhansen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/48adeca98451
Do not special-case declared-unaligned accesses on ARM. r=jseward
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 92 Branch

Comment on attachment 9234840 [details]
Bug 1723930 - Do not special-case declared-unaligned accesses on ARM. r?jseward

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a base patch for the real problem, solved in bug 1587757, q.v. for full details.
  • User impact if declined: Continued headaches for distros that build Firefox on ARMv7 systems.
  • Fix Landed on Version: FF92
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Corner case problem on ARMv7. The alternative is to let the distros live with the build headache for another full ESR cycle.
  • String or UUID changes made by this patch:
Attachment #9234840 - Flags: approval-mozilla-esr91?

Comment on attachment 9234840 [details]
Bug 1723930 - Do not special-case declared-unaligned accesses on ARM. r?jseward

Approved for 91.1esr.

Attachment #9234840 - Flags: approval-mozilla-esr91? → approval-mozilla-esr91+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: