[PATCH] D34781: Introduce a MCReloc class
Gerolf Hoflehner via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 29 17:52:31 PDT 2017
Gerolf added reviewers: ab, Gerolf.
Gerolf added a comment.
Just a first superficial review. I haven't thought about the underlying concept itself yet.
-Gerolf
================
Comment at: include/llvm/MC/MCFixup.h:156
+
+ const MCExpr *getValue() const { return Expr; }
+ void setValue(const MCExpr* Val) { Expr = Val; }
----------------
getExpr, setExpr?
================
Comment at: include/llvm/MC/MCFixup.h:159
+
+ bool isAbsolute() const { return !RefA && !SymB; }
+ MCSymbolRefExpr::VariantKind getAccessVariant() const {
----------------
Is there a better name? Absolute what?
================
Comment at: lib/MC/ELFObjectWriter.cpp:633
const MCSectionELF &FixupSection = cast<MCSectionELF>(*Fragment->getParent());
uint64_t C = Target.getConstant();
uint64_t FixupOffset = Layout.getFragmentOffset(Fragment) + Fixup.getOffset();
----------------
Same as FixedValue?
================
Comment at: lib/MC/MCAssembler.cpp:202
+ uint64_t &Value = Reloc.getConstant();
Value = 0;
if (!Expr->evaluateAsRelocatable(Target, &Layout, &Fixup)) {
----------------
Reloc.setConstant(0)?
================
Comment at: lib/MC/MCAssembler.cpp:249
+ // Let the backend check whether we need a relocation.
+ Backend.processFixupValue(*this, Fixup, Target, IsResolved);
----------------
The comments are at least confusing. From the check below it can be derived that IsResolved has the information where or not a relocation is needed already. Then there is no need to check for it.
https://reviews.llvm.org/D34781
More information about the llvm-commits
mailing list