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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/ELF/Arch/X86_64.cpp:169
+
+static bool isDirectJmpInsnOpcode(const uint8_t *Opcode) {
+  return (*Opcode == 0xe9);
----------------
tmsriram wrote:
> ruiu wrote:
> > Does "direct jmp" actually mean just J_JMP_32?
> Yes, it is J_JMP_32.  Do you want this refactored into getJmpInsnType?  
I found it a little confusing to use "direct jump" to refer only to J_JMP_32 here and used the same term to represent all J* instructions in the deleteFallThruJmpInsn function comment. But maybe you can just inline and remove this function.



================
Comment at: lld/ELF/InputSection.h:134
+  // instruction. The members below help trimming the trailing jump instruction
+  // and shrinking a section.
+  unsigned bytesDropped = 0;
----------------
Now I wonder if you can just shrink `rawData`? Then you can revert he change you made to `getSize()`.


================
Comment at: lld/ELF/OutputSections.cpp:248
+  unsigned i = 0;
+  auto nopFiller = *target->nopInstrs;
+  unsigned num = Size / nopFiller.back().size();
----------------
Please use the actual type instead of `auto`.


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

https://reviews.llvm.org/D68065