"Fixup too large" error with -fasm on PowerPC

[simonmar]

The native code generator on PowerPC can sometimes generate code that doesn't pass the assembler. The error is "Fixup too large" from the assembler.

Workaround is to use -fvia-C.

Wolfgang Thaller says this: Conditional branches on the PowerPC only have 16 bits for the displacement. I have been reluctant to fix this so far because it means either slowing down all conditional branches or actually checking the size of the generated code before deciding what branch instructions to use, which I'm afraid would add an additional pass to the NCG.

[simonmar]

See also #717

[PHO]

The problem has recurred in HEAD:

"inplace/bin/ghc-stage1" -static -prof  -H32m -O    -package-name time-1.4.0.2 -hide-all-packages -i -ilibraries/time/. -ilibraries/time/dist-install/build -ilibraries/time/dist-install/build/autogen -Ilibraries/time/dist-install/build -Ilibraries/time/dist-install/build/autogen -Ilibraries/time/include   -optP-DLANGUAGE_Rank2Types -optP-DLANGUAGE_DeriveDataTypeable -optP-DLANGUAGE_StandaloneDeriving -optP-include -optPlibraries/time/dist-install/build/autogen/cabal_macros.h -package base-4.7.0.0 -package deepseq-1.3.0.2 -package old-locale-1.0.0.5  -Wall -XHaskell2010 -XRank2Types -XDeriveDataTypeable -XStandaloneDeriving -XCPP -O2  -no-user-package-db -rtsopts      -odir libraries/time/dist-install/build -hidir libraries/time/dist-install/build -stubdir libraries/time/dist-install/build -hisuf p_hi -osuf  p_o -hcsuf p_hc -c libraries/time/./Data/Time/Format.hs -o libraries/time/dist-install/build/Data/Time/Format.p_o

/var/folders/rN/rNbpGijHEzGPi-Ux2JwMgU+++TI/-Tmp-/ghc78514_0/ghc78514_0.s:104514:0:
    Fixup of -33272 too large for field width of 16 bits

/var/folders/rN/rNbpGijHEzGPi-Ux2JwMgU+++TI/-Tmp-/ghc78514_0/ghc78514_0.s:104498:0:
    Fixup of -33636 too large for field width of 16 bits

/var/folders/rN/rNbpGijHEzGPi-Ux2JwMgU+++TI/-Tmp-/ghc78514_0/ghc78514_0.s:94635:0:
    Fixup of 33240 too large for field width of 16 bits

/var/folders/rN/rNbpGijHEzGPi-Ux2JwMgU+++TI/-Tmp-/ghc78514_0/ghc78514_0.s:94632:0:
    Fixup of 33316 too large for field width of 16 bits

/var/folders/rN/rNbpGijHEzGPi-Ux2JwMgU+++TI/-Tmp-/ghc78514_0/ghc78514_0.s:94629:0:
    Fixup of 33392 too large for field width of 16 bits

/var/folders/rN/rNbpGijHEzGPi-Ux2JwMgU+++TI/-Tmp-/ghc78514_0/ghc78514_0.s:94547:0:
    Fixup of 33504 too large for field width of 16 bits

/var/folders/rN/rNbpGijHEzGPi-Ux2JwMgU+++TI/-Tmp-/ghc78514_0/ghc78514_0.s:94486:0:
    Fixup of 33640 too large for field width of 16 bits

I found it was triggered by an extremely large function with lots of info tables. While AsmCodeGen.makeFarBranches tries to transform problematic PPC.Instr.BCC to PPC.Instr.BCCFAR, it underestimates the maximum number of instructions to jump over as it doesn't take account of the presence of info tables.

Here is my  patch to work around:

% git fetch http://github.com/phonohawk/ghc.git ticket-709

[simonmar]

Hmm, this is an unpleasant bit of code :-) hardcoded constants and guesses everywhere.

Could we not use something better than a hardcoded "5" for the info table size? We know the size of the info table for a continuation: sizeof(StgRetInfoTable), and add to that the maximum offset due to alignment.

You could also do better than length blocks: the actual number of info tables is available.

[PHO]

Replying to simonmar:

Hmm, this is an unpleasant bit of code :-) hardcoded constants and guesses everywhere.

It is unpleasant indeed. My patch makes a dirty hack even dirtier :(

Could we not use something better than a hardcoded "5" for the info table size? We know the size of the info table for a continuation: sizeof(StgRetInfoTable), and add to that the maximum offset due to alignment.

The size of StgRetInfoTable varies depending on the "way" (e.g. -prof) so we can't simply grab it from the C compiler. I think it's easy to calculate the size of each tables represented as CmmStatics.

You could also do better than length blocks: the actual number of info tables is available.

Right, but since info tables are scattered around a proc, I couldn't simply replace length blocks with the actual number of tables.

[simonmar]

Replying to PHO:

Replying to simonmar:

Could we not use something better than a hardcoded "5" for the info table size? We know the size of the info table for a continuation: sizeof(StgRetInfoTable), and add to that the maximum offset due to alignment.

The size of StgRetInfoTable varies depending on the "way" (e.g. -prof) so we can't simply grab it from the C compiler. I think it's easy to calculate the size of each tables represented as CmmStatics.

It also varies based on tablesNextToCode and other things, but you only need the upper bound here. I would add an stgRetInfoTableMaxWords :: Int somewhere, maybe CmmInfo.

You could also do better than length blocks: the actual number of info tables is available.

Right, but since info tables are scattered around a proc, I couldn't simply replace length blocks with the actual number of tables.

I don't really understand that. But I don't mind if you want to use length blocks, it's just a bit pessimistic.

[PHO]

Replying to simonmar:

It also varies based on tablesNextToCode and other things, but you only need the upper bound here. I would add an stgRetInfoTableMaxWords :: Int somewhere, maybe CmmInfo.

That would be handy though I have no idea how to generate such a constant. Do you intend to hardcode it?

You could also do better than length blocks: the actual number of info tables is available.

Right, but since info tables are scattered around a proc, I couldn't simply replace length blocks with the actual number of tables.

I don't really understand that. But I don't mind if you want to use length blocks, it's just a bit pessimistic.

I found it was my misunderstanding while I was trying to explain why it was wrong to calculate nearLimit by the number of tables. Sorry, you were right.

[simonmar]

Replying to PHO:

Replying to simonmar:

It also varies based on tablesNextToCode and other things, but you only need the upper bound here. I would add an stgRetInfoTableMaxWords :: Int somewhere, maybe CmmInfo.

That would be handy though I have no idea how to generate such a constant. Do you intend to hardcode it?

No, there are some constants already available from StgCmmLayout, e.g. stdInfoTableSizeB. You may need to add something to give you the size of a return info table though.

[PHO]

OK I revised my fix to  a series of 3 patches. Could you review it?

[marlowsd@…]

commit 48b958948cc36b3cad95e8661a642b21f120b468

Author: Simon Marlow <marlowsd@gmail.com>
Date:   Wed Jan 23 10:19:25 2013 +0000

    Tidy up: move info-table related stuff to CmmInfo
    
    Prep for #709

 compiler/cmm/CmmInfo.hs          |  155 +++++++++++++++++++++++++++++++++++++-
 compiler/cmm/CmmLayoutStack.hs   |    2 +-
 compiler/cmm/CmmParse.y          |    1 +
 compiler/codeGen/StgCmmBind.hs   |    1 +
 compiler/codeGen/StgCmmExpr.hs   |    1 +
 compiler/codeGen/StgCmmLayout.hs |  122 +-----------------------------
 compiler/codeGen/StgCmmPrim.hs   |    1 +
 compiler/ghci/DebuggerUtils.hs   |    2 +-
 8 files changed, 161 insertions(+), 124 deletions(-)

[simonmar]

Ok, I redid your first patch (see above), your other changes are fine except don't forget to take into account the effect of alignment, which could add a further (word_size - 1) bytes to the size of the info table.

[PHO]

Thanks for tidying up, but I think we don't have to worry about section alignment because PPC is an architecture where every instruction has a fixed length of word_size (unlike e.g. x86), and the size of info table is always a multiple of word_size.

[simonmar]

Ok yes, thanks for pointing that out. Would you mind rebasing your patches and I'll get them committed?

[PHO]

Sure.

[PHO]

I rebased my  second and  third patch:

% git fetch http://github.com/phonohawk/ghc.git ticket-709

[marlowsd@…]

commit 8a6e330ac9c2dde11b0641dce69bb809af6a494c

Merge: ca5d15a... 3cedbfb...
Author: Simon Marlow <marlowsd@gmail.com>
Date:   Mon Feb 4 11:19:33 2013 +0000

    Merge commit '3cedbfb49996da2f029b4a84ca39f4d21f309813'
    
    * commit '3cedbfb49996da2f029b4a84ca39f4d21f309813':
      AsmCodeGen.NcgImpl.ncgMakeFarBranches should take account of info tables (#709)
      Move AsmCodeGen.makeFarBranches to PPC.Instr (#709)

 compiler/nativeGen/AsmCodeGen.lhs |   47 ++++---------------------------------
 compiler/nativeGen/PPC/Instr.hs   |   42 ++++++++++++++++++++++++++++++++-
 2 files changed, 46 insertions(+), 43 deletions(-)

[pho@…]

commit 3cedbfb49996da2f029b4a84ca39f4d21f309813

Author: PHO <pho@cielonegro.org>
Date:   Thu Dec 20 08:13:37 2012 +0900

    AsmCodeGen.NcgImpl.ncgMakeFarBranches should take account of info tables (#709)
    
    We have to reduce the maximum number of instructions to jump over depending on the number of info tables in a proc.

 compiler/nativeGen/AsmCodeGen.lhs |    8 ++++----
 compiler/nativeGen/PPC/Instr.hs   |   16 +++++++++-------
 2 files changed, 13 insertions(+), 11 deletions(-)

[simonmar]

pushed, thanks!