[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 <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

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