Disable the SRT offset optimisation on MachO platforms
ClosedPublic

Authored by bgamari on May 20 2018, 10:38 AM.

Details

Summary

Unfortunately, this optimisation is infeasible on MachO platforms (e.g. Darwin)
due to an object format limitation. Specifically, linking fails with errors of
the form:

error: unsupported relocation with subtraction expression, symbol
'_integerzmgmp_GHCziIntegerziType_quotInteger_closure' can not be undefined
in a subtraction expression

​Apparently MachO does not permit relocations' subtraction expressions to refer
to undefined symbols. As far as I can tell this means that it is essentially
impossible to express an offset between symbols living in different compilation
units. This means that we lively can't use this optimisation on MachO platforms.

Test Plan

Validate on Darwin

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bgamari created this revision.May 20 2018, 10:38 AM
simonmar added inline comments.May 21 2018, 2:41 AM
compiler/cmm/CmmBuildInfoTables.hs
650–660

We should be able to fix this on MachO by expanding this condition. The same problem arises on ELF platforms where we can't use this relocation for dynamic symbols, I'm guess that on MachO the restriction also applies when doing static linking. It should be better to avoid the optimisation when the symbol we're trying to refer to is in a different module, than to disable the optimisation completely.

bgamari updated this revision to Diff 16496.May 21 2018, 3:48 PM

Simonmar's suggestion

simonmar added inline comments.May 22 2018, 3:50 AM
compiler/cmm/CmmBuildInfoTables.hs
650–660

This will probably expand the code size quite a bit. We'll want to be a bit more refined I think: it's fine to do this optimisation if lbl is from the current module.

bgamari updated this revision to Diff 16501.May 22 2018, 8:55 AM

Check for intra-module references

simonmar added inline comments.May 22 2018, 10:15 AM
compiler/cmm/CLabel.hs
989

let's add a case for LocalBlockLabel here

989

I think we should check for external names too: nameModule name == this_mod (you'll need to pass in this_mod, but it's readily available at the call site).

bgamari updated this revision to Diff 16508.May 22 2018, 7:37 PM

Simonmar's suggestion

bgamari marked 2 inline comments as done.EditedMay 22 2018, 7:37 PM

Done.

simonmar accepted this revision.May 23 2018, 4:23 AM

Thanks for doing this!

compiler/cmm/CmmBuildInfoTables.hs
124

You can remove this exception now. We still use this representation on MachO, see below where we could note the exception.

226

could note the MachO exception here

This revision is now accepted and ready to land.May 23 2018, 4:23 AM

Build error though:

148	compiler/cmm/CLabel.hs:993:5: error:
149	    Not in scope: data constructor ‘LocalInfoLabel’
150	    Perhaps you meant ‘LocalInfoTable’ (line 389)
151	    |
152	993 |     LocalInfoLabel   -> True
153	    |     ^^^^^^^^^^^^^^
This revision was automatically updated to reflect the committed changes.