On Fri, 2007-01-19 at 11:25 -0600, Anton Korobeynikov wrote: > > Changes in directory llvm/lib/ExecutionEngine/JIT: > > JITEmitter.cpp updated: 1.124 -> 1.125 > --- > Log message: > > Adding disassembler interface and external hook to udis86 library.
Some notes below. > > > --- > Diffs of the changes: (+14 -1) > > JITEmitter.cpp | 15 ++++++++++++++- > 1 files changed, 14 insertions(+), 1 deletion(-) > > > Index: llvm/lib/ExecutionEngine/JIT/JITEmitter.cpp > diff -u llvm/lib/ExecutionEngine/JIT/JITEmitter.cpp:1.124 > llvm/lib/ExecutionEngine/JIT/JITEmitter.cpp:1.125 > --- llvm/lib/ExecutionEngine/JIT/JITEmitter.cpp:1.124 Tue Dec 19 16:43:32 2006 > +++ llvm/lib/ExecutionEngine/JIT/JITEmitter.cpp Fri Jan 19 11:25:17 2007 > @@ -27,6 +27,7 @@ > #include "llvm/Target/TargetJITInfo.h" > #include "llvm/Target/TargetMachine.h" > #include "llvm/Support/Debug.h" > +#include "llvm/Support/Disassembler.h" > #include "llvm/Support/MutexGuard.h" > #include "llvm/ADT/Statistic.h" > #include "llvm/System/Memory.h" > @@ -847,7 +848,7 @@ > } > > // Update the GOT entry for F to point to the new code. > - if(MemMgr.isManagingGOT()) { > + if (MemMgr.isManagingGOT()) { > unsigned idx = > getJITResolver(this).getGOTIndexForAddr((void*)BufferBegin); > if (((void**)MemMgr.getGOTBase())[idx] != (void*)BufferBegin) { > DOUT << "GOT was out of date for " << (void*)BufferBegin > @@ -864,6 +865,18 @@ > << ": " << (FnEnd-FnStart) << " bytes of text, " > << Relocations.size() << " relocations\n"; > Relocations.clear(); > + > + DOUT << "Disassembled code:\n" > +#if defined(__i386__) > + << disassembleBuffer(FnStart, FnEnd-FnStart, > + Disassembler::X86_32, (uint32_t)FnStart); I'm not thrilled about having this system specific #if/#elif/#else/#endif in the JITEmitter. I would prefer to see the disassemblBuffer function become part of lib/System instead of lib/Support and DTRT for any combination of supported platforms. The list of disassembled architectures will grow over time (PPC/ARM/Sparc) and I'd rather not have this sprinkled across the users of disassembleBuffer but in one location in lib/System. > +#elif defined(__amd64__) || defined(__x86_64__) > + << disassembleBuffer(FnStart, FnEnd-FnStart, > + Disassembler::X86_64, (uint32_t)FnStart); > +#else > + << "N/A\n"; That's not very descriptive message. How about "architecture not supported". > +#endif > + > return false; > } > > > > > _______________________________________________ > llvm-commits mailing list > llvm-commits@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits _______________________________________________ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits