[PATCH] D94627: [PowerPC][PC Rel] Implement option to omit P10 instructions from stubs

Albion Fung via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 25 15:17:45 PST 2021


Conanap added a comment.

Addressed comments



================
Comment at: lld/ELF/Config.h:74
+// For --power10-stub
+enum class P10Stub { Default, No };
+
----------------
amyk wrote:
> We have a "yes", but does it need to be here, too?
After a bit of discussion, since we don't have a concrete implementation for the "yes" option yet, the consensus is to keep it out for now. I'll remove it from the help message as well.


================
Comment at: lld/ELF/Driver.cpp:776
+  }
+
+  return P10Stub::Default;
----------------
amyk wrote:
> Do we need to handle `power10_stubs`?
That option is handled by the variable `NoP10`; I'll add a comment to make that more clear.


================
Comment at: lld/ELF/Thunks.cpp:920
   const int64_t offset = computeOffset();
-  write32(buf + 0, 0xf8410018);                         // std  r2,24(r1)
+  write32(buf + 0, 0xf8410018);                           // std  r2,24(r1)
   // The branch offset needs to fit in 26 bits.
----------------
amyk wrote:
> Unrelated change?
This change was made so that the commented instruction would better line up with the new instructions added for better readablity. 


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

https://reviews.llvm.org/D94627



More information about the llvm-commits mailing list