fix: make BiFManager builtin-index hash independent of the C++ standard library#417
Open
TsaiGaggery wants to merge 1 commit into
Open
fix: make BiFManager builtin-index hash independent of the C++ standard library#417TsaiGaggery wants to merge 1 commit into
TsaiGaggery wants to merge 1 commit into
Conversation
…rd library
BiFManagerCommon::getHash() used std::hash<std::string>, whose value is
implementation-defined and differs across C++ standard libraries. BiFManager-bin
bakes the hashes of builtin names into OCLBiFImpl.h as `case <hash>ULL:` labels at
build time, and BiFManagerHandler::GetDepList recomputes getHash() for each
referenced function at run time to find its section.
When the host BiFManager-bin and the libigc runtime are linked against different
standard libraries (e.g. a libstdc++ host tool generating the header while libigc
is built with libc++, the usual situation when cross-compiling IGC for Android),
the generated case labels and the runtime lookups disagree for every name. The
switch always falls through to {-1}, findAllBuiltins() collects nothing, LinkBiF()
links no built-in bodies, and every __spirv_* reference is reported "undefined
reference to ..." -> backend compiler failure (CL_BUILD_PROGRAM_FAILURE). Native
single-stdlib builds are unaffected, which is why this only surfaces in
heterogeneous-stdlib / cross builds.
Use a fixed 64-bit FNV-1a hash so the generated labels and the runtime lookups
always agree regardless of the standard library. getHash() is defined once in
BiFManagerCommon and compiled into both the host tool and the runtime, so a single
definition keeps them consistent.
Signed-off-by: Gaggery Tsai <gaggery.tsai@intel.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
BiFManagerCommon::getHash()usesstd::hash<std::string>, whose result isimplementation-defined and differs across C++ standard libraries.
BiFManager-binbakes the hashes of builtin names into the generated
OCLBiFImpl.hascase <hash>ULL:labels at build time, andBiFManagerHandler::GetDepListrecomputes
getHash()for each referenced function at run time to find its section.When the host
BiFManager-binand thelibigcruntime are linked againstdifferent standard libraries -- the usual situation when cross-compiling IGC for
Android (a libstdc++ host tool generating the header while
libigcis built withlibc++) -- the generated
caselabels and the runtime lookups disagree for everyname. The switch always falls through to
{-1},findAllBuiltins()collectsnothing,
LinkBiF()links no built-in bodies, and every__spirv_*reference isreported
undefined reference to ...-> backend compiler failure(
CL_BUILD_PROGRAM_FAILURE). Native single-stdlib builds are unaffected, which is whythis only surfaces in heterogeneous-stdlib / cross builds.
Fix
Use a fixed 64-bit FNV-1a hash so the generated labels and the runtime lookups always
agree regardless of the C++ standard library.
getHash()is defined once inBiFManagerCommonand compiled into both the host tool and the runtime.Validation
Reproduced cross-compiling IGC (LLVM 17.0.6) for Android x86_64: host
BiFManager-binlinked against libstdc++,
libigc.soagainst libc++. Before this patch,ocloc compileof any kernel using a builtin failed with
undefined reference to _Z33__spirv_BuiltInGlobalInvocationIdi. Binary proof: thelibstdc++ hash of that symbol appeared as an immediate inside
libigc.sowhile thelibc++ value the runtime actually computes appeared 0 times. After this patch the
FNV-1a value matches on both sides,
findAllBuiltinscollects the builtins,oclocreports
Build succeeded., and OpenVINO GPU inference runs end to end.