Re: [PATCH] Add minimal Hexagon target - First in a series of patches - linux-user changes + linux-user/hexagon + skeleton of target/hexagon - Files in target/hexagon/imported are from another project and therefore do not conform to qemu coding standards

From: Laurent Vivier <laurent@vivier.eu>
To: "Taylor Simpson" <tsimpson@quicinc.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"riku.voipio@iki.fi" <riku.voipio@iki.fi>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Subject: Re: [PATCH] Add minimal Hexagon target - First in a series of patches - linux-user changes + linux-user/hexagon + skeleton of target/hexagon - Files in target/hexagon/imported are from another project and therefore do not conform to qemu coding standards
Date: Wed, 20 Nov 2019 15:14:38 +0100	[thread overview]
Message-ID: <daae6a71-a5ba-21f7-ee57-2493d298ac52@vivier.eu> (raw)
In-Reply-To: <BYAPR02MB48867C73CDE23CA70FEE5369DE4F0@BYAPR02MB4886.namprd02.prod.outlook.com>

Le 20/11/2019 à 13:58, Taylor Simpson a écrit :
> Is there a precedent for this?  I'm OK with DEBUG_HEX, but I assumed reviewers wouldn't approve
> #ifdef FIXME
> #define DEBUG_HEX
> #endif

For instance, in target/mips/translate.c you have:

     42
     43 #define MIPS_DEBUG_DISAS 0
     44
...
   2603 #define LOG_DISAS(...)  \
   2604     do { \
   2605         if (MIPS_DEBUG_DISAS) { \
   2606             qemu_log_mask(CPU_LOG_TB_IN_ASM, ## __VA_ARGS__);\
   2607         }  \
   2608     } while (0)
   2609
...

  10121     LOG_DISAS("mftr (reg %d u %d sel %d h %d)\n", rt, u, sel,
h);
...

For the linux-user part, I don't think you need the DEBUG_HEX traces.

Thanks,
Laurent
> 
> -----Original Message-----
> From: Richard Henderson <richard.henderson@linaro.org>
> Sent: Wednesday, November 20, 2019 3:02 AM
> To: Laurent Vivier <laurent@vivier.eu>; Taylor Simpson <tsimpson@quicinc.com>; Philippe Mathieu-Daudé <philmd@redhat.com>; riku.voipio@iki.fi; qemu-devel@nongnu.org
> Subject: Re: [PATCH] Add minimal Hexagon target - First in a series of patches - linux-user changes + linux-user/hexagon + skeleton of target/hexagon - Files in target/hexagon/imported are from another project and therefore do not conform to qemu coding standards
> 
> 
> On 11/20/19 9:33 AM, Laurent Vivier wrote:
>> Le 20/11/2019 à 05:48, Taylor Simpson a écrit :
>>> For the general DEBUG_HEX messages, I think the trace infrastructure isn't quite what I'm looking for.
>>>
>>> Here's a sample of what it prints
>>> Start packet: pc = 0x4002f0
>>> Packet committed: pc = 0x4002f0
>>> Regs written
>>>         r18 = -69420 (0xfffef0d4)
>>>         r19 = -69412 (0xfffef0dc)
>>> Stores
>>>         memd[0xfffef0b0] = 0 (0x0000000000000000) Next PC = 0x4002f8
>>> Exec counters: pkt = 17, insn = 24, hvx = 0
> 
> For something like this, I'd keep DEBUG_HEX.
> 
>>>     if (qemu_loglevel_maks(CPU_LOG_EXEC)) {
> 
> CPU_LOG_EXEC already has a specific structure, listing the TranslationBlocks that are executed.  It shouldn't be hijacked for something else.
> 
> If you really want a runtime flag for this, we should add a new CPU_LOG_* flag.
> 
> 
> r~
> 