Opened 4 years ago

Closed 4 years ago

#10433 closed bug (fixed)

Fix load/store barriers in pre-ARMv7 builds

Reported by: thoughtpolice Owned by:
Priority: high Milestone: 8.0.1
Component: Runtime System Version:
Keywords: Cc: simonmar, erikd
Operating System: Unknown/Multiple Architecture: arm
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: #10244 Differential Rev(s): Phab:D1704
Wiki Page:

Description

As pointed out in #10244 and Phab:D894, the fix we committed for this problem isn't 100% correct - from 753b156dc6b0c38b106c390952750fb800bf27e7:

#elif arm_HOST_ARCH && defined(arm_HOST_ARCH_PRE_ARMv7)
    // TODO FIXME: This case probably isn't totally correct - just because we
    // use a pre-ARMv7 toolchain (e.g. to target an old Android device), doesn't
    // mean the binary won't run on a newer ARMv7 system - in which case it
    // needs a proper barrier. So we should rethink this
    //  - Reid
    __asm__ __volatile__ ("" : : : "memory");

This is a reminder to fix this.

Change History (11)

comment:1 Changed 4 years ago by thoughtpolice

Owner: changed from simonmar to rwbarton

comment:2 Changed 4 years ago by rwbarton

Owner: rwbarton deleted

I won't be getting around to fixing this any time soon.

The easy, conservative thing to do would be to set NOSMP if arm_HOST_ARCH_PRE_ARMv7 is defined. The fancy way would be to detect at runtime whether we are on a system that supports memory barriers and patch them in if so (like Linux's altinstructions). Probably too fancy...

comment:3 Changed 4 years ago by thoughtpolice

Milestone: 7.12.18.0.1

Milestone renamed

comment:4 Changed 4 years ago by erikd

Cc: erikd added

comment:5 Changed 4 years ago by bgamari

Differential Rev(s): Phab:D1704
Status: newpatch

comment:6 Changed 4 years ago by bgamari

Is there anyone who could test Phab:D1704?

comment:7 Changed 4 years ago by bgamari

Status: patchinfoneeded

comment:8 Changed 4 years ago by bgamari

Status: infoneedednew

Reid has confirmed that this builds on Phab:D1704 with a cross-compiler for ARMv5.

comment:9 Changed 4 years ago by bgamari

Status: newmerge

comment:10 Changed 4 years ago by Ben Gamari <ben@…>

In 9fe7d20e/ghc:

Ensure that we don't produce code for pre-ARMv7 without barriers

We are unable to produce load/store barriers for pre-ARMv7 targets.
Phab:D894 added dummy cases to SMP.h for these barriers to prevent the
build from failing under the assumption that there are no SMP-capable
devices of this vintage. However, #10433 points out that it is more
correct to simply set NOSMP for such targets.

Tested By: rwbarton

Test Plan: Validate

Reviewers: erikd, rwbarton, austin

Reviewed By: rwbarton

Subscribers: thomie

Differential Revision: https://phabricator.haskell.org/D1704

GHC Trac Issues: #10433

comment:11 Changed 4 years ago by bgamari

Resolution: fixed
Status: mergeclosed
Note: See TracTickets for help on using tickets.