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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits