Opened 6 years ago

Last modified 5 weeks ago

#2725 new task

Remove Hack in compiler/nativeGen/X86/CodeGen.hs

Reported by: clemens Owned by: thoughtpolice
Priority: low Milestone: 7.12.1
Component: Compiler (NCG) Version: 6.11
Keywords: codegen Cc: dterei, simonmar, erikd
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Revisions:

Description (last modified by igloo)

Remove the hack around Line 3914 labeled with:

    -- HACK: On x86_64 binutils<2.17 is only able to generate PC32
    -- relocations, hence we only get 32-bit offsets in the jump
    -- table. As these offsets are always negative we need to properly
    -- sign extend them to 64-bit. This hack should be removed in
    -- conjunction with the hack in PprMach.hs/pprDataItem once
    -- binutils 2.17 is standard.

This bug is intended for house keeping.

Attachments (1)

test-64bit-pc-rel-reloc.sh (621 bytes) - added by erikd 7 weeks ago.
Test for 64 bit pc-relative relocations

Download all attachments as: .zip

Change History (20)

comment:1 Changed 6 years ago by igloo

  • Description modified (diff)
  • difficulty set to Unknown
  • Milestone set to 6.12 branch

comment:2 Changed 5 years ago by igloo

  • Milestone changed from 6.12 branch to 6.12.3

comment:3 Changed 5 years ago by igloo

  • Milestone changed from 6.12.3 to 6.14.1
  • Priority changed from normal to low

comment:4 Changed 4 years ago by igloo

  • Milestone changed from 7.0.1 to 7.0.2

comment:5 Changed 4 years ago by igloo

  • Milestone changed from 7.0.2 to 7.2.1

comment:6 Changed 4 years ago by dterei

  • Cc dterei added
  • Type of failure set to None/Unknown

comment:7 Changed 4 years ago by igloo

  • Milestone changed from 7.2.1 to 7.4.1

comment:8 Changed 3 years ago by igloo

  • Milestone changed from 7.4.1 to 7.6.1
  • Priority changed from low to lowest

comment:9 Changed 3 years ago by igloo

  • Milestone changed from 7.6.1 to 7.6.2

comment:10 Changed 2 years ago by igloo

  • Milestone changed from 7.6.2 to 7.8.1
  • Priority changed from lowest to high

This was opened 4 years ago; let's see if we can remove the hack and close it now.

nb, it's now in nativeGen/X86/CodeGen.hs.

comment:11 Changed 19 months ago by thoughtpolice

  • Cc simonmar added
  • Owner changed from clemens to thoughtpolice

comment:12 Changed 12 months ago by thoughtpolice

  • Keywords codegen added
  • Milestone changed from 7.8.3 to 7.10.1
  • Priority changed from high to normal

comment:13 Changed 4 months ago by thoughtpolice

  • Milestone changed from 7.10.1 to 7.12.1

Moving to 7.12.1 milestone; if you feel this is an error and should be addressed sooner, please move it back to the 7.10.1 milestone.

Changed 7 weeks ago by erikd

Test for 64 bit pc-relative relocations

comment:14 Changed 7 weeks ago by erikd

I have just (2015/03/03) written a shell script (attached) that tests for binutils support for 64 bit pc-relative relocatons. The test passes on *all* modern linux systems, but fails on Mac OSX (Mavericks) and OpenBSD 5.5 (current is 5.6). The failure is one OpenBSD says:

linkasm.s:3: Error: can not do 8 byte pc-relative relocation

OpenBSD has GNU binutils 2.15 but they are unlikely to upgrade to a later version because later versions are under GPLv3 instead of GPLv2. However, they may at some stage switch to some other linker.

OSX has the linker from LLVM which theoretically could be fixed.

comment:15 Changed 7 weeks ago by erikd

  • Cc erikd added

comment:16 Changed 6 weeks ago by rwbarton

  • Summary changed from Remove Hack in compiler/nativeGen/MachCodeGen.hs to Remove Hack in compiler/nativeGen/X86/CodeGen.hs

I would not be surprised if it was better (equal or faster performance and smaller object file size) to use 32-bit offsets anyways, has anyone done a performance test?

comment:17 Changed 6 weeks ago by nomeata

  • Priority changed from normal to low

As austin writes in Phab:D694, we are not going to able to change that in the near future anyways. Lowering severity.

comment:18 Changed 6 weeks ago by rwbarton

Oh, I see the jump table entries are currently padded to 64 bits anyways. So then it should be slightly better to use a 64-bit relocation instead, but maybe it would be even better to pack the entries into 32 bits instead.

comment:19 Changed 5 weeks ago by thomie

  • Type changed from bug to task

From Phab:D694: "If there still are broken binutils around, then maybe we should simply leave the current code (which works) in for another 10 years."

Note: See TracTickets for help on using tickets.