[PATCH] D103539: WIP: try to repair RISCV handling of paired relocations
Jessica Clarke via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 2 12:07:49 PDT 2021
jrtc27 added inline comments.
================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp:396
return;
+
MCContext &Ctx = Asm.getContext();
----------------
Please don't "fix" unrelated whitespace
================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp:56
const MCExpr *Expr = Fixup.getValue();
+
// Determine the type of the relocation
----------------
Ditto
================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp:156-161
+ std::advance(iterator, RI);
+
+ Relocs[RI].Type = ELF::R_RISCV_ADD8;
+ Relocs.emplace(iterator, Relocs[RI].Offset, nullptr, ELF::R_RISCV_SUB8, 0,
+ nullptr, 0);
+
----------------
I'm confused why this doesn't overwrite RI again with an R_RISCV_SUB8?
================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp:160
+ Relocs.emplace(iterator, Relocs[RI].Offset, nullptr, ELF::R_RISCV_SUB8, 0,
+ nullptr, 0);
+
----------------
This approach is also going to give quadratic complexity. Is there a reason we can't emplace_back? If so, can a stable sort post-processing step not fix that more efficiently (asymptotically)?
================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFObjectWriter.cpp:163
+ ++RI, ++RE;
+ break;
+ }
----------------
Please no comma operators outside of `for`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D103539/new/
https://reviews.llvm.org/D103539
More information about the llvm-commits
mailing list