[PATCH] D68065: Propeller: LLD Support for Basic Block Sections

[David Li via Phabricator via llvm-commits]

davidxl added a comment.

In D68065#1864338 <https://reviews.llvm.org/D68065#1864338>, @MaskRay wrote:

> In D68065#1863669 <https://reviews.llvm.org/D68065#1863669>, @ruiu wrote:
>
> > Here is a summary of today's discussion:
> >
> > - this seems a straightforward implementation of your proposal
> > - this patch introduces a new notion of "Jump Relocations" to represent data related to rewriting jump instructions at end of sections. Even though the Jump Relocations are somewhat similar to existing Relocations, but they are maintained in different arrays, and they are handled and consumed by different code locations. So maybe we should avoid calling it a "relocation" because "jump relocations" aren't relocations in object files?
>
>
> So you had a discussion about this:)
>
> I attended a LLVM Social yesterday and asked some people about their feelings. We are still dubious whether doing all the heavy lifting on the linker side is the right approach. Disassembly at linker side may give some short term benefits, but the interaction with debug info/asynchronous unwind tables/symbol information is muddy. The improvement is fixed. Linkers don't really understand the semantics, so the available optimizations are rather limited. In a long term, this can cause some maintenance burden. I can think of several problems:
>
> - Needs a psABI defined relocation type. The relocation type will probably have no use other than this very specific optimization.
> - Needs non-trivial work to port to COFF.
> - Another target may be a very different story.
> - This duplicates some work of MachineBlockPlacement.
> - The code will be duplicated on lld side may be a lot. `emitNops` can be reused with AsmPrinter. Branch terminators optimization may be reused with BranchFolding. etc It the current MachineFunction interface does not allow  sharable MachineBasicBlocks with other MachineFunctions. Fix it.


On a very high level, any major new functionality will likely to share the same kinds of 'cons' as you mentioned.  Though I feel  a lot of concerns have been discussed in the RFC, I think the concerns you mentioned are worth addressing.

1. most of the relocation types are introduced for a single purpose
2. having a different binary format in mind is a good thing --- so that the code can eventually be in the form that is mostly format agnostic.
3. similarly true for different targets.
4. this does not dup machine block placement in many ways: a) MBP does not see the most precise profile due to  prior transformations b) MBP itself does tailDup which results in CFG cange hat needs to be relaid out with  PLO c) lack of Interproc capaclity
5. for the last concern, I think it is case by case, and should be discussed and addressed in patch review.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68065/new/

https://reviews.llvm.org/D68065