[llvm-commits] CVS: llvm/lib/ExecutionEngine/JIT/JITEmitter.cpp

[Reid Spencer]

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 at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits