Fix PPC NCG after blockID patch
ClosedPublic

Authored by trommler on Nov 2 2017, 11:44 AM.

Details

Summary

Commit rGHC8b007ab assigns the same label to the first basic block
of a proc and to the proc entry point. This violates the PPC 64-bit ELF
v. 1.9 and v. 2.0 ABIs and leads to duplicate symbols.

This patch fixes duplicate symbols caused by block labels

In commit rGHCd7b8da1 an info table label is generated from a block id.
Getting the entry label from that info label leads to an undefined
symbol because a suffix "_entry" that is not present in the block label.

To fix that issue add a new info table label flavour for labels
derived from block ids. Converting such a label with toEntryLabel
produces the original block label.

Fixes Trac #14311

Test Plan

./validate

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.
trommler created this revision.Nov 2 2017, 11:44 AM

@bgamari I believe you should be having a look at this :-(

bgamari requested changes to this revision.Nov 3 2017, 11:12 AM

Can we have a Note explaining why this is necessary?

compiler/cmm/CLabel.hs
379

Perhaps we should rename this to EntryInfoTable and/or make the comment more specific?

395

Can we reword this a bit more clearly? Perhaps,

Like 'InfoTable' but for a proc-point block instead of a closure entry-point.

This revision now requires changes to proceed.Nov 3 2017, 11:12 AM
trommler edited the summary of this revision. (Show Details)Nov 3 2017, 2:06 PM
trommler edited the summary of this revision. (Show Details)Nov 4 2017, 6:17 AM
trommler updated this revision to Diff 14557.Nov 4 2017, 1:07 PM
  • Break up long lines
  • Make block info tables local and simplify
  • Simplify and always use infoTblLbl
  • Add note Proc-point block entry-point
trommler planned changes to this revision.Nov 4 2017, 1:10 PM
trommler marked an inline comment as done.

I'd like to fix powerpc64le, too.

trommler edited the summary of this revision. (Show Details)Nov 4 2017, 2:15 PM
trommler edited the summary of this revision. (Show Details)
trommler updated this revision to Diff 14560.Nov 5 2017, 2:48 AM
  • Fix duplicate labels on powerpc64le
trommler added inline comments.Nov 5 2017, 2:58 AM
compiler/cmm/CLabel.hs
379

Entry and InfoTable are externally visible, the new BlockInfoTable is not. Perhaps my comment below should say "Like LocalInfoTable ..."?

bgamari added inline comments.Nov 5 2017, 9:32 AM
compiler/cmm/CLabel.hs
379

That might help. That being said, it would be even better if the Note explicitly pointed out that they are not externally-visible.

In general I think part of the confusion in this area is that we are using "block" to refer to a "local block". Strictly speaking an entry block is also a "block", yet it is externally visible. This overloading makes documenting this much harder than it in principle should be.

trommler updated this revision to Diff 14561.Nov 5 2017, 11:46 AM
  • Improve comments
trommler marked an inline comment as done.Nov 5 2017, 11:48 AM
austin resigned from this revision.Nov 6 2017, 3:52 PM
This revision is now accepted and ready to land.Nov 9 2017, 3:41 PM
This revision was automatically updated to reflect the committed changes.