[Bug Fix] Enabling custom op name without adding stack frame locations #11
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes two related issues in
PopulateXlaOpMetadata
that prevented proper usage ofCustomOpNameMetaData
:Fix 1: Support custom op names without stack frame locations
This will enable user to change the name of the op using
CustomOpNameMetaData
without necessarily having to add stack frame locations. Example of usage (if we pass 0 formax_stack_depth
field it will just prepend custom prefix to the current op name):Additionaly, this will guard the
AddStackFrameLocations()
function from passing invalid number for themax_stack_depth
. If we were to pass number that is <= 0, we would get a segmentation fault due to improper iterator dereferencing (which could've happen before this change).The
AddStackFrameLocations()
function instack_frame_index_builder.cpp
uses reverse iterators and assumes at least one iteration occurs:Fix 2: Prevent scope from overwriting custom metadata
Problem:
Even when users set custom metadata via
_set_xla_custom_op_name_prefix()
, thenmeta.scope
field was unconditionally overwriting the customop_name_prefix
. This affected operations likeadd
andmul
which have scope set (e.g.,aten::add.3
), resulting in loss of user-provided semantic location information.Changes:
if (!nmeta.scope.empty())
toelse if (!nmeta.scope.empty())
Precedence hierarchy (now correctly implemented):
SetUserMetadata
APIs) - highest priority