amyk added a comment.
A few general comments.
================
Comment at: lld/ELF/Config.h:74
+// For --power10-stub
+enum class P10Stub { Default, No };
+
----------------
We have a "yes", but does it need to be here, too?
================
Comment at: lld/ELF/Driver.cpp:763
+static P10Stub getP10StubOpt(opt::InputArgList &args) {
+
----------------
Add documentation for the function.
================
Comment at: lld/ELF/Driver.cpp:776
+ }
+
+ return P10Stub::Default;
----------------
Do we need to handle `power10_stubs`?
================
Comment at: lld/ELF/Options.td:450
+def power10_stubs_eq:
+ J<"power10-stubs=">, HelpText<"Enables Power10 instr in all stubs without
options, "
+ "options override previous flags."
----------------
nit: maybe use the full word `instructions` here and in the following lines.
================
Comment at: lld/ELF/Options.td:452
+ "options override previous flags."
+ "auto: Default"
+ "no: No Power10 instr in stubs"
----------------
Describe the default behaviour?
================
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.
----------------
Unrelated change?
================
Comment at: lld/ELF/Thunks.cpp:925
} else if (isInt<34>(offset)) {
- const uint64_t paddi = PADDI_R12_NO_DISP |
- (((offset >> 16) & 0x3ffff) << 32) |
- (offset & 0xffff);
- writePrefixedInstruction(buf + 4, paddi); // paddi r12, 0, func@pcrel, 1
- write32(buf + 12, MTCTR_R12); // mtctr r12
- write32(buf + 16, BCTR); // bctr
+ int inst2;
+ if (config->Power10Stub == P10Stub::No) {
----------------
Can we maybe try to have a more descriptive name than `inst2`? (here and in the
other places)
================
Comment at: lld/test/ELF/ppc64-toc-call-to-pcrel.s:23
+
+## The point of this test is to make sure that when a function with TOC access
# a local function with st_other=1, a TOC save stub is inserted.
----------------
No need for the extra `#`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94627/new/
https://reviews.llvm.org/D94627
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
