[PATCH] D56637: [llvm-objdump] - Cleanup the code. NFCI.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 14 03:13:14 PST 2019


jhenderson added inline comments.


================
Comment at: llvm-objdump.cpp:571-572
     return getRelocationValueString(ELF32BE, Rel, Result);
-  auto *ELF64BE = cast<ELF64BEObjectFile>(Obj);
-  return getRelocationValueString(ELF64BE, Rel, Result);
+  if (auto *ELF64LE = dyn_cast<ELF64LEObjectFile>(Obj))
+    return getRelocationValueString(ELF64LE, Rel, Result);
+  return getRelocationValueString(cast<ELF64BEObjectFile>(Obj), Rel, Result);
----------------
grimar wrote:
> jhenderson wrote:
> > grimar wrote:
> > > jhenderson wrote:
> > > > Any particular reason you've moved this one? The previous order I think was just as good.
> > > Previous order was by endianness. Isn't it a bit strange? I think grouping by address size is a much more common way in LLVM.
> > If you can confirm that address order is much more common, then this is fine. If it is not significantly more common, then I think we should avoid unnecessary change.
> I am OK to omit this change from this patch. Other style inconsistencies that were addressed look a lot more critical to me.
I think so. I did a quick search, and its inconsistent across LLVM.


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

https://reviews.llvm.org/D56637





More information about the llvm-commits mailing list