Re: [Mesa-dev] [PATCH 1/3] gallivm: Don't use raw_debug_ostream for dissasembling

[Jose Fonseca]

On 23/07/15 17:06, Jose Fonseca wrote:
On 20/07/15 21:39, Jose Fonseca wrote:
On 20/07/15 18:35, Tom Stellard wrote:
All LLVM API calls that require an ostream object have been removed from
the disassemble() function, so we don't need to use this class to wrap
_debug_printf() we can just call this function directly.
---
  src/gallium/auxiliary/gallivm/lp_bld_debug.cpp | 27
+++++++++++++-------------
  1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/src/gallium/auxiliary/gallivm/lp_bld_debug.cpp
b/src/gallium/auxiliary/gallivm/lp_bld_debug.cpp
index 405e648..ec88f33 100644
--- a/src/gallium/auxiliary/gallivm/lp_bld_debug.cpp
+++ b/src/gallium/auxiliary/gallivm/lp_bld_debug.cpp
@@ -123,7 +123,7 @@ lp_debug_dump_value(LLVMValueRef value)
   * - http://blog.llvm.org/2010/04/intro-to-llvm-mc-project.html
   */
  static size_t
-disassemble(const void* func, llvm::raw_ostream & Out)
+disassemble(const void* func)
  {
     const uint8_t *bytes = (const uint8_t *)func;

@@ -141,7 +141,8 @@ disassemble(const void* func, llvm::raw_ostream &
Out)
     char outline[1024];

     if (!D) {
-      Out << "error: couldn't create disassembler for triple " <<
Triple << "\n";
+      _debug_printf("error: couldn't create disassembler for triple
%s\n",
+                    Triple.c_str());
        return 0;
     }

@@ -155,13 +156,13 @@ disassemble(const void* func, llvm::raw_ostream
& Out)
         * so that between runs.
         */

-      Out << llvm::format("%6lu:\t", (unsigned long)pc);
+      _debug_printf("%6lu:\t", (unsigned long)pc);

        Size = LLVMDisasmInstruction(D, (uint8_t *)bytes + pc, extent
- pc, 0, outline,
                                     sizeof outline);

        if (!Size) {
-         Out << "invalid\n";
+         _debug_printf("invalid\n");
           pc += 1;
           break;
        }
@@ -173,10 +174,10 @@ disassemble(const void* func, llvm::raw_ostream
& Out)
        if (0) {
           unsigned i;
           for (i = 0; i < Size; ++i) {
-            Out << llvm::format("%02x ", bytes[pc + i]);
+            _debug_printf("%02x ", bytes[pc + i]);
           }
           for (; i < 16; ++i) {
-            Out << "   ";
+            _debug_printf("   ");
           }
        }

@@ -184,9 +185,9 @@ disassemble(const void* func, llvm::raw_ostream &
Out)
         * Print the instruction.
         */

-      Out << outline;
+      _debug_printf("%*s", Size, outline);

-      Out << "\n";
+      _debug_printf("\n");

        /*
         * Stop disassembling on return statements, if there is no
record of a
@@ -206,13 +207,12 @@ disassemble(const void* func, llvm::raw_ostream
& Out)
        pc += Size;

        if (pc >= extent) {
-         Out << "disassembly larger than " << extent << "bytes,
aborting\n";
+         _debug_printf("disassembly larger than %ull bytes,
aborting\n", extent);
           break;
        }
     }

-   Out << "\n";
-   Out.flush();
+   _debug_printf("\n");

     LLVMDisasmDispose(D);

@@ -229,9 +229,8 @@ disassemble(const void* func, llvm::raw_ostream &
Out)

  extern "C" void
  lp_disassemble(LLVMValueRef func, const void *code) {
-   raw_debug_ostream Out;
-   Out << LLVMGetValueName(func) << ":\n";
-   disassemble(code, Out);
+   _debug_printf("%s:\n", LLVMGetValueName(func));
+   disassemble(code);
  }




Series looks good AFAICT.

Reviewed-by: Jose Fonseca <jfons...@vmware.com>

Tom,

This broke the profile build.  I fixed with commit
d6b50ba980b733a82fefe2a0f115635a359c445f, but only then I realized that
this change actually causes regression of functionality.

The debug stream was being used so that we would write the assembly to
/tmp/perf-XXXXX.map.asm files, so that we could later annotate the
assembly intrusctions with the profile hits (via the
bin/perf-annotate-jit).

But now the assembly is being redirected into the stderr, hence
perf-annotate-jit won't get anything from /tmp/perf-XXXXX.map.asm.

What was your motivation for this change? General cleanup, or is the
llvm::raw_ostream causing you problems?

I think that, if I don't reinstate the debug_stream, then I'll need to
replace it with STL streams (std::stringstream and std::ostream).

Jose

Ping?

I wanted to make sure I didn't cause you trouble, but if I don't get any 
answer I'll just assume you don't care either way, and revert the 
change, as it saves me the effort of trying to replace debug_stream with 
something else.

Jose
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev