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

Tue Apr 7 12:31:55 PDT 2020

MaskRay added inline comments.

================
Comment at: lld/ELF/Writer.cpp:2118
+  // sections.
+  //auto startOptBBJumpTime = system_clock::now();
+  optimizeBasicBlockJumps();
----------------
tmsriram wrote:
> MaskRay wrote:
> > tmsriram wrote:
> > > MaskRay wrote:
> > > > MaskRay wrote:
> > > > > Delete unrelated comments.
> > > > `optimizeBasicBlockJumps` calls assignAddresses, which was only called in finalizeAddressDependentContent.
> > > > 
> > > > We hope assignAddresses caller are grouped together (if in.symTab needs to be finalized first, please add a comment). Can you move this pass immediately before (or after) finalizeAddressDependentContent?
> > > Correct me if I am wrong but finalizing in.symtab before we optimize seems important, I added a comment here.  If we relax jumps going forward, we would definitely need to know how far they are.
> > > Run after in.symTab is finalized.
> > 
> > Why is important?
> Modified comment.
I am still not sure this is correct. .symtab and .strtab and potential .shstrtab are placed after everything else. The internal representation does not use the contents of `.symtab` at all.

Though, you already committed this patch. We probably don't want to change here more to cause churn.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68065