[PATCH] D158280: [jitlink/rtdydl][checker] Construct disassembler at every decodeInst

[Lang Hames via Phabricator via llvm-commits]

lhames added a comment.

Minor comments aside this is looking great. Once these are fixed I think that this is ready to land.



================
Comment at: llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h:626-629
   /// Check whether the given target flags are set for this Symbol.
   bool hasTargetFlags(TargetFlagsType Flags) const {
     return static_cast<TargetFlagsType>(TargetFlags) & Flags;
   }
----------------
This can be removed now that `getTargetFlags` has been added.


================
Comment at: llvm/include/llvm/ExecutionEngine/RuntimeDyldChecker.h:86-89
     /// Constructor for symbols/sections with content.
     MemoryRegionInfo(ArrayRef<char> Content, JITTargetAddress TargetAddress)
         : ContentPtr(Content.data()), Size(Content.size()),
           TargetAddress(TargetAddress) {}
----------------
This constructor should be removed so that clients are forced to specify a flags value explicitly, even if it's just the default.


================
Comment at: llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldChecker.cpp:801-802
+
+  assert(TheTriple.isThumb() == (0x1 & getTargetFlag(Symbol)) &&
+         "The Symbol is Thumb but the Triple does not support it");
+
----------------
This assertion should be removed: 0x1 is only the thumb flag for arm, not other architectures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158280