Skip to content

Fix the clang issue due to empty structure in the Tracing Layer #432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
pratikbariintel wants to merge 3 commits into
base: master
Choose a base branch
from
+26 −21
Open

Fix the clang issue due to empty structure in the Tracing Layer #432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b6835dd
Fix the clang issue due to empty structure in the Tracing Layer
pratikbariintel Feb 25, 2026
580bab1
Fix the clang issue due to empty structure in the Tracing Layer
pratikbariintel Feb 25, 2026
7fad9e5
Fix the clang issue due to empty structure in the Tracing Layer
pratikbariintel Feb 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 2 additions & 11 deletions include/layers/zel_tracing_register_cb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2721,25 +2721,16 @@ typedef void (ZE_APICALL *zer_pfnTranslateIdentifierToDeviceHandleCb_t)(
void** ppTracerInstanceUserData
);

///////////////////////////////////////////////////////////////////////////////
/// @brief Callback function parameters for zerGetDefaultContext
/// @details Each entry is a pointer to the parameter passed to the function;
/// allowing the callback the ability to modify the parameter's value

typedef struct _zer_get_default_context_params_t
{
} zer_get_default_context_params_t;


///////////////////////////////////////////////////////////////////////////////
/// @brief Callback function-pointer for zerGetDefaultContext
/// @param[in] params Parameters passed to this instance
/// @param[in] params Parameters passed to this instance (NULL for parameter-less functions)
/// @param[in] result Return value
/// @param[in] pTracerUserData Per-Tracer user data
/// @param[in,out] ppTracerInstanceUserData Per-Tracer, Per-Instance user data

typedef void (ZE_APICALL *zer_pfnGetDefaultContextCb_t)(
zer_get_default_context_params_t* params,
void* params,
Copy link

@joshuaranjan joshuaranjan Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need void * params.
If empty params, if possible, lets skip this member variable

Copy link
Contributor Author

@pratikbariintel pratikbariintel Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently all the callbacks defined have the same pattern. In case this field is not required we have to define a new callback pattern.

Copy link
Contributor

@rwmcguir rwmcguir Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this data type is used in the exported function call:

ZE_APIEXPORT ze_result_t ZE_APICALL
zelTracerGetDefaultContextRegisterCallback(
zel_tracer_handle_t hTracer,
zel_tracer_reg_t callback_type,
zer_pfnGetDefaultContextCb_t pfnGetDefaultContextCb
);

So while it won't break immediate compilation or exported function ABI, the public struct type zer_pfnGetDefaultContextCb_t is changing in it's struct members. Thinking through this out loud.. The struct remains the same size, and void* also remains the same size.

The problem is the callback itself, the user must typecast it to match our callback data type, i.e. you see this yourself in the test content. It breaks how the user must define the actual callback API. This is my fear that, yes, this breaks ABI compatibility. Can instead of going this route, perhaps insert a placeholder void member; or something into the actual data structure, to avoid the clang warning?

Copy link

@joshuaranjan joshuaranjan Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can instead of going this route, perhaps insert a placeholder void member; or something into the actual data structure, to avoid the clang warning?

That would change the size of the structure ? Which might again be an ABI issue.

Copy link

@joshuaranjan joshuaranjan Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a workable solution which would not break ABI.
The solutions we discussed seems to break ABI one way or the other.

@rwmcguir @pratikbariintel

Copy link
Contributor Author

@pratikbariintel pratikbariintel Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we consider the addition of a placeholder in the previous structure (zer_get_default_context_params_t), then this will also change its size. However, since this structure is not used anywhere inside the code, it might not be throwing the ABI issue.

If we go by this approach, I think it is better to name the placeholder something like 'reserved', so that the user doesn't think of using the same.

Copy link
Contributor

@vishnu-khanth vishnu-khanth Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function pointer signature is changed in tracing header.
Just thinking about whether this breaks backward compatibility

ze_context_handle_t result,
void* pTracerUserData,
void** ppTracerInstanceUserData
Expand Down
18 changes: 15 additions & 3 deletions scripts/templates/tracing/trc_setters.h.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,28 +44,40 @@ typedef struct _zel_tracer_handle_t *zel_tracer_handle_t;
%for obj in tbl['functions']:
<%
ret_type = obj['return_type']
%>///////////////////////////////////////////////////////////////////////////////
param_lines = th.make_param_lines(n, tags, obj, format=["type*", "name"])
%>\
%if param_lines:
///////////////////////////////////////////////////////////////////////////////
/// @brief Callback function parameters for ${th.make_func_name(n, tags, obj)}
/// @details Each entry is a pointer to the parameter passed to the function;
/// allowing the callback the ability to modify the parameter's value

typedef struct _${th.make_pfncb_param_type(n, tags, obj)}
{
%for line in th.make_param_lines(n, tags, obj, format=["type*", "name"]):
%for line in param_lines:
${line};
%endfor
} ${th.make_pfncb_param_type(n, tags, obj)};

%endif

///////////////////////////////////////////////////////////////////////////////
/// @brief Callback function-pointer for ${th.make_func_name(n, tags, obj)}
/// @param[in] params Parameters passed to this instance
/// @param[in] params Parameters passed to this instance\
%if not param_lines:
(NULL for parameter-less functions)\
%endif

/// @param[in] result Return value
/// @param[in] pTracerUserData Per-Tracer user data
/// @param[in,out] ppTracerInstanceUserData Per-Tracer, Per-Instance user data

typedef void (${X}_APICALL *${th.make_pfncb_type(n, tags, obj)})(
%if param_lines:
${th.make_pfncb_param_type(n, tags, obj)}* params,
%else:
void* params,
%endif
${ret_type} result,
void* pTracerUserData,
void** ppTracerInstanceUserData
Expand Down
8 changes: 6 additions & 2 deletions scripts/templates/tracing/trcddi.cpp.mako
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,19 +58,23 @@ namespace tracing_layer
);

// capture parameters
${th.make_pfncb_param_type(n, tags, obj)} tracerParams = {
%if not is_void_params:
${th.make_pfncb_param_type(n, tags, obj)} tracerParams = {
&${",\n &".join(params_list)}
%endif
};
%endif

tracing_layer::APITracerCallbackDataImp<${th.make_pfncb_type(n, tags, obj)}> apiCallbackData;

${N}_GEN_PER_API_CALLBACK_STATE(apiCallbackData, ${th.make_pfncb_type(n, tags, obj)}, ${th.get_callback_table_name(n, tags, obj)}, ${th.make_pfncb_name(n, tags, obj)});


return tracing_layer::APITracerWrapperImp<${ret_type}>(context.${n}DdiTable.${th.get_table_name(n, tags, obj)}.${th.make_pfn_name(n, tags, obj)},
%if not is_void_params:
&tracerParams,
%else:
nullptr,
%endif
apiCallbackData.apiOrdinal,
apiCallbackData.prologCallbacks,
apiCallbackData.epilogCallbacks\
Expand Down
4 changes: 1 addition & 3 deletions source/layers/tracing/zer_trcddi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,16 +125,14 @@ namespace tracing_layer
ZE_HANDLE_TRACER_RECURSION(context.zerDdiTable.Global.pfnGetDefaultContext);

// capture parameters
zer_get_default_context_params_t tracerParams = {
};

tracing_layer::APITracerCallbackDataImp<zer_pfnGetDefaultContextCb_t> apiCallbackData;

ZER_GEN_PER_API_CALLBACK_STATE(apiCallbackData, zer_pfnGetDefaultContextCb_t, Global, pfnGetDefaultContextCb);


return tracing_layer::APITracerWrapperImp<ze_context_handle_t>(context.zerDdiTable.Global.pfnGetDefaultContext,
&tracerParams,
nullptr,
apiCallbackData.apiOrdinal,
apiCallbackData.prologCallbacks,
apiCallbackData.epilogCallbacks);
Expand Down
4 changes: 2 additions & 2 deletions test/loader_tracing_layer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ namespace
data->incrementZerPrologueCallCount("zerTranslateIdentifierToDeviceHandle");
}

static void zerGetDefaultContextPrologueCallback(zer_get_default_context_params_t *params, ze_context_handle_t result, void *pTracerUserData, void **ppTracerInstanceUserData)
static void zerGetDefaultContextPrologueCallback(void *params, ze_context_handle_t result, void *pTracerUserData, void **ppTracerInstanceUserData)
{
TracingData *data = static_cast<TracingData *>(pTracerUserData);
data->incrementZerPrologueCallCount("zerGetDefaultContext");
Expand All @@ -446,7 +446,7 @@ namespace
data->incrementZerEpilogueCallCount("zerTranslateIdentifierToDeviceHandle");
}

static void zerGetDefaultContextEpilogueCallback(zer_get_default_context_params_t *params, ze_context_handle_t result, void *pTracerUserData, void **ppTracerInstanceUserData)
static void zerGetDefaultContextEpilogueCallback(void *params, ze_context_handle_t result, void *pTracerUserData, void **ppTracerInstanceUserData)
{
TracingData *data = static_cast<TracingData *>(pTracerUserData);
data->incrementZerEpilogueCallCount("zerGetDefaultContext");
Expand Down
Loading