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

[Jose Fonseca]

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 <jfonseca at 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