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

[Stefan Pintilie via Phabricator via llvm-commits]

stefanp added a comment.

Just nits this time around.



================
Comment at: lld/ELF/Driver.cpp:765
+// instructions in stubs.
+static bool getP10StubOpt(opt::InputArgList &args) {
+
----------------
Conanap wrote:
> For this function here, I realize we can inline all the ifs into a giant return statement - is there any opinions on this? I thought the if statements might make this a bit more readable, but if it is preferred that there is only 1 return statement I can make that change as well.
Personally I'm happy with it like this. I'm not a fan of huge if statements but I could be persuaded otherwise depending on what other reviewers have to say...


================
Comment at: lld/ELF/Options.td:451
+  J<"power10-stubs=">, HelpText<
+                     "Enables Power10 instsructions in all stubs without options, "
+                     "options override previous flags."
----------------
nit:
instsructions -> instructions 


================
Comment at: lld/ELF/Thunks.cpp:936
+        const uint64_t addi = ADDI_R12_TO_R2_NO_DISP | (tocOffset & 0xffff);
+        write32(buf + 4, addi); // addi r12, 2, offset
+        nextInstOffset = 8;
----------------
nit:
addi r12, 2, offset -> addi r12, r2, offset


================
Comment at: lld/test/ELF/ppc64-pcrel-call-to-extern.s:110
+
+## .plt[2] - 0x10010010 = 0x10030158 - 0x10010010 = 0x20148 = 131416
 # CHECK-LABEL: <__plt_pcrel_callee_global_stother0>:
----------------
nit:
Same thing here. Please fix the comment.
`0x20148 = 131400` and not `131416`


================
Comment at: lld/test/ELF/ppc64-pcrel-call-to-extern.s:135
+
+## .plt[3] - 0x10020010 = 0x10030160 - 0x10020010 = 0x10150 = 65888
 # CHECK-LABEL: <__plt_pcrel_callee_global_stother1>:
----------------
nit:
Please fix these comments.
`0x10150` does not equal `65888`.


================
Comment at: lld/test/ELF/ppc64-pcrel-call-to-extern.s:160
+
+## .plt[4] - 0x10030010 = 0x10030168 - 0x10030010 = 0x150 = 360
 # CHECK-LABEL: <__plt_pcrel_callee_global_TOC>:
----------------
nit:
This comment too.
```
0x10030168 - 0x10030010 = 0x158
0x150 = 336
```
I'm not going to comment on all of these. Please take a look at the comments where numbers have changed and make sure that they are correct.



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

https://reviews.llvm.org/D94627