From 8491fb0bb421e2f006d0217b2950a3d9348b4a79 Mon Sep 17 00:00:00 2001 From: Didier Nadeau Date: Tue, 21 Jun 2016 15:48:52 -0400 Subject: [PATCH 1/5] Modifications to ROCm-GDB to use internal breakpoints instead of SIGUSR2 to notify GDB when the offloaded program on the GPU hits a breakpoint. --- gdb-7.8/amd/include/CommunicationControl.h | 3 +- gdb-7.8/gdb/breakpoint.c | 59 ++++++++++++++++++++++ gdb-7.8/gdb/breakpoint.h | 2 + gdb-7.8/gdb/hsail-fifo-control.c | 19 +++++++ gdb-7.8/gdb/hsail-fifo-control.h | 2 + 5 files changed, 84 insertions(+), 1 deletion(-) diff --git a/gdb-7.8/amd/include/CommunicationControl.h b/gdb-7.8/amd/include/CommunicationControl.h index 2f0515dc..a8b3ca8e 100644 --- a/gdb-7.8/amd/include/CommunicationControl.h +++ b/gdb-7.8/amd/include/CommunicationControl.h @@ -31,7 +31,8 @@ typedef enum HSAIL_COMMAND_MOMENTARY_BREAKPOINT, // Set an HSAIL momentary breakpoint (which is automatically deleted) HSAIL_COMMAND_CONTINUE, // Continue the inferior process HSAIL_COMMAND_SET_LOGGING, // Configure the logging in the Agent - HSAIL_COMMAND_SET_ISA_DUMP // Configure dumping of ISA + HSAIL_COMMAND_SET_ISA_DUMP, // Configure dumping of ISA + HSAIL_COMMAND_SET_HSABP // Use breakpoint instead of SIGUSR2 to stop the library } HsailCommand; typedef enum diff --git a/gdb-7.8/gdb/breakpoint.c b/gdb-7.8/gdb/breakpoint.c index 23685e1f..d0e52a81 100644 --- a/gdb-7.8/gdb/breakpoint.c +++ b/gdb-7.8/gdb/breakpoint.c @@ -303,6 +303,9 @@ static struct breakpoint_ops longjmp_breakpoint_ops; breakpoints. */ struct breakpoint_ops bkpt_breakpoint_ops; +/* The breapoint_ops structure to be used in HSA library breakpoint */ +struct breakpoint_ops hsa_breakpoint_ops; + /* Breakpoints set on probes. */ static struct breakpoint_ops bkpt_probe_breakpoint_ops; @@ -13613,6 +13616,40 @@ bkpt_resources_needed (const struct bp_location *bl) return 1; } +static enum print_stop_action +hsa_bkpt_print_it (bpstat bs) +{ + struct breakpoint *b; + const struct bp_location *bl; + int bp_temp; + struct ui_out *uiout = current_uiout; + + gdb_assert (bs->bp_location_at != NULL); + + bl = bs->bp_location_at; + b = bs->breakpoint_at; + + bp_temp = b->disposition == disp_del; + if (bl->address != bl->requested_address) + breakpoint_adjustment_warning (bl->requested_address, + bl->address, + b->number, 1); + annotate_breakpoint (b->number); + if (bp_temp) + ui_out_text (uiout, "\nTemporary breakpoint triggered by GPU Kernel Breakpoint"); + else + ui_out_text (uiout, "\nStopped on GPU Breakpoint"); + if (ui_out_is_mi_like_p (uiout)) + { + ui_out_field_string (uiout, "reason", + async_reason_lookup (EXEC_ASYNC_BREAKPOINT_HIT)); + ui_out_field_string (uiout, "disp", bpdisp_text (b->disposition)); + } + ui_out_text (uiout, ", "); + + return PRINT_SRC_AND_LOC; +} + static enum print_stop_action bkpt_print_it (bpstat bs) { @@ -16598,6 +16635,11 @@ initialize_breakpoint_ops (void) ops->print_mention = bkpt_print_mention; ops->print_recreate = bkpt_print_recreate; + /* The HSA breakpoint structure that will be hit when the GPU hits a breakpoint. */ + ops = &hsa_breakpoint_ops; + *ops = bkpt_breakpoint_ops; + ops->print_it = hsa_bkpt_print_it; + /* Ranged breakpoints. */ ops = &ranged_breakpoint_ops; *ops = bkpt_breakpoint_ops; @@ -16761,6 +16803,23 @@ initialize_breakpoint_ops (void) ops->breakpoint_hit = dprintf_breakpoint_hit; } +void +create_hsa_gpu_breakpoint(char *location) +{ + create_breakpoint(get_current_arch(), + location, + NULL, 0, NULL, 1, + 0, bp_breakpoint, + 0, + pending_break_support, + &hsa_breakpoint_ops, + 1, + 1, /* Is enabled. */ + 1, /* Internal breakpoint. */ + 0); /* No flags */ + +} + /* Chain containing all defined "enable breakpoint" subcommands. */ static struct cmd_list_element *enablebreaklist = NULL; diff --git a/gdb-7.8/gdb/breakpoint.h b/gdb-7.8/gdb/breakpoint.h index f40698c5..6314bb2d 100644 --- a/gdb-7.8/gdb/breakpoint.h +++ b/gdb-7.8/gdb/breakpoint.h @@ -1294,6 +1294,8 @@ extern int create_breakpoint (struct gdbarch *gdbarch, char *arg, int enabled, int internal, unsigned flags); +extern void create_hsa_gpu_breakpoint (char *location); + extern void insert_breakpoints (void); extern int remove_breakpoints (void); diff --git a/gdb-7.8/gdb/hsail-fifo-control.c b/gdb-7.8/gdb/hsail-fifo-control.c index 03cea4d1..fef99157 100644 --- a/gdb-7.8/gdb/hsail-fifo-control.c +++ b/gdb-7.8/gdb/hsail-fifo-control.c @@ -136,6 +136,9 @@ static void hsail_validate_command_packet(const HsailCommandPacket packet) valid =1; } break; + case HSAIL_COMMAND_SET_HSABP: + valid = 1; + break; case HSAIL_COMMAND_UNKNOWN: valid = 0; break; @@ -317,6 +320,22 @@ static void hsail_push_command(HsailCommandPacket packet) gdb_assert(bytes_written == sizeof(HsailCommandPacket)); } +/* This function is called to tell the agent to use the internal + * breakpoint instead of SIGUSR2 to stop to notify GDB of a GPU + * kernel breakpoint. + */ +void +hsail_enqueue_enable_hsa_breakpoint(void) +{ + HsailCommandPacket hsa_bp_packet; + + memset(&hsa_bp_packet, 0, sizeof(hsa_bp_packet)); + + hsa_bp_packet.m_command = HSAIL_COMMAND_SET_HSABP; + + hsail_push_command(hsa_bp_packet); +} + /* * Create a breakpoint packet and put it to the fifo. * We could pass the hsail breakpoint request here but we also need to send PC diff --git a/gdb-7.8/gdb/hsail-fifo-control.h b/gdb-7.8/gdb/hsail-fifo-control.h index be302f5a..07c4419b 100644 --- a/gdb-7.8/gdb/hsail-fifo-control.h +++ b/gdb-7.8/gdb/hsail-fifo-control.h @@ -36,6 +36,8 @@ void hsail_free_command_buffer(void); void hsail_flush_breakpoint_command_buffer(void); +void hsail_enqueue_enable_hsa_breakpoint(void); + void hsail_enqueue_continue_dispatch_packet(void); void hsail_enqueue_create_breakpoint_request_buffer(const HsailBreakpointRequest* request); From ddcd2303162fc6cae99d3c6eb956ba1b54ae7273 Mon Sep 17 00:00:00 2001 From: Didier Nadeau Date: Tue, 21 Jun 2016 15:53:08 -0400 Subject: [PATCH 2/5] Missing change in previous commit to use breakpoint instead of SIGUSR2. --- gdb-7.8/gdb/hsail-tdep.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/gdb-7.8/gdb/hsail-tdep.c b/gdb-7.8/gdb/hsail-tdep.c index 57d12689..bf0e7d80 100644 --- a/gdb-7.8/gdb/hsail-tdep.c +++ b/gdb-7.8/gdb/hsail-tdep.c @@ -106,6 +106,8 @@ void hsail_linux_initialize_stg2(void) * */ struct ui_out* uiout = current_uiout; + char * hsaBreakpointLocation = " TriggerStop"; + int fd = 0; gdb_assert(NULL != uiout); @@ -127,6 +129,10 @@ void hsail_linux_initialize_stg2(void) g_hsail_fifo_descriptor = fd; } + create_hsa_gpu_breakpoint(hsaBreakpointLocation); + + hsail_enqueue_enable_hsa_breakpoint(); + /* * The shared memory for the dbe binary is created in the agent */ From 59a82fcda88aef1c9061322d5735085a89084366 Mon Sep 17 00:00:00 2001 From: Didier Nadeau Date: Mon, 27 Jun 2016 12:54:31 -0400 Subject: [PATCH 3/5] Removal of the codepath that uses SIGUSR2. It now uses a breakpoint inserted in the library to trigger a stop on a GPU breakpoint. --- gdb-7.8/amd/include/CommunicationControl.h | 3 +-- gdb-7.8/gdb/breakpoint.c | 9 ++++++++- gdb-7.8/gdb/hsail-fifo-control.c | 19 ------------------- gdb-7.8/gdb/hsail-fifo-control.h | 2 -- gdb-7.8/gdb/hsail-tdep.c | 7 ++++--- 5 files changed, 13 insertions(+), 27 deletions(-) diff --git a/gdb-7.8/amd/include/CommunicationControl.h b/gdb-7.8/amd/include/CommunicationControl.h index a8b3ca8e..f92773e5 100644 --- a/gdb-7.8/amd/include/CommunicationControl.h +++ b/gdb-7.8/amd/include/CommunicationControl.h @@ -31,8 +31,7 @@ typedef enum HSAIL_COMMAND_MOMENTARY_BREAKPOINT, // Set an HSAIL momentary breakpoint (which is automatically deleted) HSAIL_COMMAND_CONTINUE, // Continue the inferior process HSAIL_COMMAND_SET_LOGGING, // Configure the logging in the Agent - HSAIL_COMMAND_SET_ISA_DUMP, // Configure dumping of ISA - HSAIL_COMMAND_SET_HSABP // Use breakpoint instead of SIGUSR2 to stop the library + HSAIL_COMMAND_SET_ISA_DUMP // Configure dumping of ISA } HsailCommand; typedef enum diff --git a/gdb-7.8/gdb/breakpoint.c b/gdb-7.8/gdb/breakpoint.c index d0e52a81..3c0a20ed 100644 --- a/gdb-7.8/gdb/breakpoint.c +++ b/gdb-7.8/gdb/breakpoint.c @@ -16806,8 +16806,15 @@ initialize_breakpoint_ops (void) void create_hsa_gpu_breakpoint(char *location) { + /* A copy to a local buffer is used, as + * location might be in a read-only memory + * and create_breakpoint takes a non constant + * char *. + */ + char buffer[30]; + strncpy(buffer, location,30); create_breakpoint(get_current_arch(), - location, + buffer, NULL, 0, NULL, 1, 0, bp_breakpoint, 0, diff --git a/gdb-7.8/gdb/hsail-fifo-control.c b/gdb-7.8/gdb/hsail-fifo-control.c index fef99157..03cea4d1 100644 --- a/gdb-7.8/gdb/hsail-fifo-control.c +++ b/gdb-7.8/gdb/hsail-fifo-control.c @@ -136,9 +136,6 @@ static void hsail_validate_command_packet(const HsailCommandPacket packet) valid =1; } break; - case HSAIL_COMMAND_SET_HSABP: - valid = 1; - break; case HSAIL_COMMAND_UNKNOWN: valid = 0; break; @@ -320,22 +317,6 @@ static void hsail_push_command(HsailCommandPacket packet) gdb_assert(bytes_written == sizeof(HsailCommandPacket)); } -/* This function is called to tell the agent to use the internal - * breakpoint instead of SIGUSR2 to stop to notify GDB of a GPU - * kernel breakpoint. - */ -void -hsail_enqueue_enable_hsa_breakpoint(void) -{ - HsailCommandPacket hsa_bp_packet; - - memset(&hsa_bp_packet, 0, sizeof(hsa_bp_packet)); - - hsa_bp_packet.m_command = HSAIL_COMMAND_SET_HSABP; - - hsail_push_command(hsa_bp_packet); -} - /* * Create a breakpoint packet and put it to the fifo. * We could pass the hsail breakpoint request here but we also need to send PC diff --git a/gdb-7.8/gdb/hsail-fifo-control.h b/gdb-7.8/gdb/hsail-fifo-control.h index 07c4419b..be302f5a 100644 --- a/gdb-7.8/gdb/hsail-fifo-control.h +++ b/gdb-7.8/gdb/hsail-fifo-control.h @@ -36,8 +36,6 @@ void hsail_free_command_buffer(void); void hsail_flush_breakpoint_command_buffer(void); -void hsail_enqueue_enable_hsa_breakpoint(void); - void hsail_enqueue_continue_dispatch_packet(void); void hsail_enqueue_create_breakpoint_request_buffer(const HsailBreakpointRequest* request); diff --git a/gdb-7.8/gdb/hsail-tdep.c b/gdb-7.8/gdb/hsail-tdep.c index bf0e7d80..462d0205 100644 --- a/gdb-7.8/gdb/hsail-tdep.c +++ b/gdb-7.8/gdb/hsail-tdep.c @@ -106,7 +106,7 @@ void hsail_linux_initialize_stg2(void) * */ struct ui_out* uiout = current_uiout; - char * hsaBreakpointLocation = " TriggerStop"; + char * hsaBreakpointLocation = "TriggerStop"; int fd = 0; @@ -129,10 +129,11 @@ void hsail_linux_initialize_stg2(void) g_hsail_fifo_descriptor = fd; } + /* Place the breakpoint in the GPUDebugSDK library + * that will be hit when a gpu breakpoint is hit. + */ create_hsa_gpu_breakpoint(hsaBreakpointLocation); - hsail_enqueue_enable_hsa_breakpoint(); - /* * The shared memory for the dbe binary is created in the agent */ From 626cc7219295b434a2288e1c527959bb02a81679 Mon Sep 17 00:00:00 2001 From: Didier Nadeau Date: Mon, 27 Jun 2016 16:51:41 -0400 Subject: [PATCH 4/5] -Refactoring to give more explicit names to functions and variables. -Updating the name of the function used to stop from TriggerStop to TriggerGPUBreakpointStop to follow the refactoring in GPUDebugSDK. --- gdb-7.8/gdb/breakpoint.c | 14 +++++++------- gdb-7.8/gdb/breakpoint.h | 2 +- gdb-7.8/gdb/hsail-tdep.c | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/gdb-7.8/gdb/breakpoint.c b/gdb-7.8/gdb/breakpoint.c index 3c0a20ed..46bacbb3 100644 --- a/gdb-7.8/gdb/breakpoint.c +++ b/gdb-7.8/gdb/breakpoint.c @@ -303,8 +303,8 @@ static struct breakpoint_ops longjmp_breakpoint_ops; breakpoints. */ struct breakpoint_ops bkpt_breakpoint_ops; -/* The breapoint_ops structure to be used in HSA library breakpoint */ -struct breakpoint_ops hsa_breakpoint_ops; +/* The breakpoint_ops structure to be used in HSA library breakpoint */ +struct breakpoint_ops gpu_breakpoint_trigger_ops; /* Breakpoints set on probes. */ static struct breakpoint_ops bkpt_probe_breakpoint_ops; @@ -13617,7 +13617,7 @@ bkpt_resources_needed (const struct bp_location *bl) } static enum print_stop_action -hsa_bkpt_print_it (bpstat bs) +gpu_bkpt_trigger_print_it (bpstat bs) { struct breakpoint *b; const struct bp_location *bl; @@ -16636,9 +16636,9 @@ initialize_breakpoint_ops (void) ops->print_recreate = bkpt_print_recreate; /* The HSA breakpoint structure that will be hit when the GPU hits a breakpoint. */ - ops = &hsa_breakpoint_ops; + ops = &gpu_breakpoint_trigger_ops; *ops = bkpt_breakpoint_ops; - ops->print_it = hsa_bkpt_print_it; + ops->print_it = gpu_bkpt_trigger_print_it; /* Ranged breakpoints. */ ops = &ranged_breakpoint_ops; @@ -16804,7 +16804,7 @@ initialize_breakpoint_ops (void) } void -create_hsa_gpu_breakpoint(char *location) +create_hsa_gpu_breakpoint_trigger(char *location) { /* A copy to a local buffer is used, as * location might be in a read-only memory @@ -16819,7 +16819,7 @@ create_hsa_gpu_breakpoint(char *location) 0, bp_breakpoint, 0, pending_break_support, - &hsa_breakpoint_ops, + &gpu_breakpoint_trigger_ops, 1, 1, /* Is enabled. */ 1, /* Internal breakpoint. */ diff --git a/gdb-7.8/gdb/breakpoint.h b/gdb-7.8/gdb/breakpoint.h index 6314bb2d..0b4e13ec 100644 --- a/gdb-7.8/gdb/breakpoint.h +++ b/gdb-7.8/gdb/breakpoint.h @@ -1294,7 +1294,7 @@ extern int create_breakpoint (struct gdbarch *gdbarch, char *arg, int enabled, int internal, unsigned flags); -extern void create_hsa_gpu_breakpoint (char *location); +extern void create_hsa_gpu_breakpoint_trigger (char *location); extern void insert_breakpoints (void); diff --git a/gdb-7.8/gdb/hsail-tdep.c b/gdb-7.8/gdb/hsail-tdep.c index 462d0205..53134bd3 100644 --- a/gdb-7.8/gdb/hsail-tdep.c +++ b/gdb-7.8/gdb/hsail-tdep.c @@ -106,7 +106,7 @@ void hsail_linux_initialize_stg2(void) * */ struct ui_out* uiout = current_uiout; - char * hsaBreakpointLocation = "TriggerStop"; + char * hsaBreakpointLocation = "TriggerGPUBreakpointStop"; int fd = 0; @@ -132,7 +132,7 @@ void hsail_linux_initialize_stg2(void) /* Place the breakpoint in the GPUDebugSDK library * that will be hit when a gpu breakpoint is hit. */ - create_hsa_gpu_breakpoint(hsaBreakpointLocation); + create_hsa_gpu_breakpoint_trigger(hsaBreakpointLocation); /* * The shared memory for the dbe binary is created in the agent From 27e827a922ad8e1240c490b194350a4f38b8add4 Mon Sep 17 00:00:00 2001 From: Didier Nadeau Date: Thu, 30 Jun 2016 12:10:41 -0400 Subject: [PATCH 5/5] Modification of the notification when a GPU Breakpoint is hit. --- gdb-7.8/gdb/breakpoint.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/gdb-7.8/gdb/breakpoint.c b/gdb-7.8/gdb/breakpoint.c index 46bacbb3..f2b8ef9b 100644 --- a/gdb-7.8/gdb/breakpoint.c +++ b/gdb-7.8/gdb/breakpoint.c @@ -13636,18 +13636,17 @@ gpu_bkpt_trigger_print_it (bpstat bs) b->number, 1); annotate_breakpoint (b->number); if (bp_temp) - ui_out_text (uiout, "\nTemporary breakpoint triggered by GPU Kernel Breakpoint"); + ui_out_text (uiout, "\nTemporary breakpoint triggered by GPU Kernel Breakpoint\n"); else - ui_out_text (uiout, "\nStopped on GPU Breakpoint"); + ui_out_text (uiout, "\nStopped on GPU Breakpoint\n"); if (ui_out_is_mi_like_p (uiout)) { ui_out_field_string (uiout, "reason", async_reason_lookup (EXEC_ASYNC_BREAKPOINT_HIT)); ui_out_field_string (uiout, "disp", bpdisp_text (b->disposition)); } - ui_out_text (uiout, ", "); - return PRINT_SRC_AND_LOC; + return PRINT_NOTHING; } static enum print_stop_action