[PATCH] D79977: [ELF][PPC64] Synthesize _savegpr[01]_{14..31} and _restgpr[01]_{14..31}

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/Arch/PPC64.cpp:133
+void elf::addPPC64SaveRestore() {
+  static uint32_t savegpr0[20], restgpr0[21], savegpr1[19], restgpr1[19];
+  constexpr uint32_t blr = 0x4e800020, mtlr_0 = 0x7c0803a6;
----------------
sfertile wrote:
> MaskRay wrote:
> > sfertile wrote:
> > > Since we are always writing the entire routine into the buffer anyway why not just hard code the array contents so we don't have to compute it at runtime?
> > We need to define symbols `_savegpr*` and `_restgpr*`, so a loop is needed. There is little saving if we move instructions alone to a constant array.
> The savings mostly come from making the 4 loops we have now (and the 2 that need to be added later for the fpr save/restore routines) nearly identical. We can then write a helper function or lambda and call that 6 time instead of having 6 loops once we add support for  `_savefpr*` and `_restfpr*` as well. With a minor tweak I believe  the helper would also work for the vector save restore routines as well.
> 
> Something like:
> 
> ```
> Helper(uint32_t *Buffer, uint32_t BufferSize, StringRef Prefix) {
>   int r = findFirstOf(prefix);  // returns the register number corresponding to the first symbol we need to define.
>   // Any referenced symbols are defined, nothing to do.
>   if (r == 32)
>   return;
> 
>   int Value = 0;
>   const int Offset = 4 * (r - 14);
>   for (r < 32; ++r) {
>    Name = FormatName(prefix, r)
>     addOptional(Name, Value, defined);
>     Value += 4;
>   }
>   // No need to check if defined is empty becuase we would have returned already if no symbols need to be defiend.
>   addSection(Buffer + Offset, Buffer + Size, defined);
> }
> 
> 
> ```
> 
> Its longer and more verbose then one of the for loops, but I think its much easier to follow the logic, and then  it only takes 6 lines like:
> 
> ```
> Helper(savegpr0, ArraySize(savegpr0), "__savegpr0_");
> ```
> 
> Its more lines of code total, since defining the arrays adds more lines then we save with the replacing the for loops with the helper but I think its much more approachable for a reader.
Thanks for the suggestion. I have found a way to simplify the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79977