[PATCH] D72344: [LLD][ELF][ARM][AArch64] Only round up ThunkSection Size when large OS.

[Peter Smith via Phabricator via llvm-commits]

peter.smith marked 3 inline comments as done.
peter.smith added a comment.

Thanks for the comments. I've decided to rework this In light of it not actually fixing the problem in Linux. In the allyesconfig build the overall .text OutputSection is larger than thunkSectionSpacing, but the assert was for in effect an InputSectionDescription within .text so I've had to think again. I'm nearly ready to submit again, just got to test in on the kernel itself first.



================
Comment at: lld/ELF/SyntheticSections.cpp:3387
+    // a call to assignAddresses().
+    roundUpSizeForErrata = true;
 }
----------------
grimar wrote:
> does the following look better?
> 
> ```
> roundUpSizeForErrata = (config->fixCortexA53Errata843419 || config->fixCortexA8) &&
>       os->size >= target->getThunkSectionSpacing();
> ```
> 
> But, basing on the comment which says "os->size is stable" (I've not debug the code, sorry),
> can't you just get rid of `roundUpSizeForErrata` and do the following?
> 
> ```
> size_t ThunkSection::getSize() const {
>   if ((config->fixCortexA53Errata843419 || config->fixCortexA8) &&
>       this->parent->size >= target->getThunkSectionSpacing())
>     return alignTo(size, 4096);
>   return size;
> }
> ```
Unfortunately not as getSize() is called when calculating the addresses and this resets this->parent->size. I ended up with a different size reported when doing assignOffsets.


================
Comment at: lld/ELF/SyntheticSections.h:1058
   size_t size = 0;
+  bool roundUpSizeForErrata = false;
 };
----------------
grimar wrote:
> This needs a comment probably.
Agreed, will do so by reference.


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

https://reviews.llvm.org/D72344