Skip to content

[SPIR-V] Rework duplicate tracker and tracking of IR entities and types to improve compile-time performance #130605

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.

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

Merged

Conversation

VyacheslavLevytskyy
Copy link
Contributor

@VyacheslavLevytskyy VyacheslavLevytskyy commented Mar 10, 2025

This PR is to thoroughly rework duplicate tracker implementation and tracking of IR entities and types. These are legacy parts of the project resulting in an extremely bloated intermediate representation and computational delays due to inefficient data flow and structure choices.

Main results of the rework:

  1. Improved compile-time performance. The reference binary LLVM IR used to measure speed gains in [SPIR-V] Overhaul module analysis to improve translation speed and simplify the underlying logics #120415 shows ~x5 speed up also after this PR. The timing before this PR is ~42s and after this PR it's ~7.5s. In total this PR and the previous overhaul of the module analysis in [SPIR-V] Overhaul module analysis to improve translation speed and simplify the underlying logics #120415 results in ~x25 speed improvement.
$ time llc -O0 -mtriple=spirv64v1.6-unknown-unknown _group_barrier_phi.bc -o 1 --filetype=obj

real    0m7.545s
user    0m6.685s
sys     0m0.859s
  1. Less bloated intermediate representation of internal translation steps. Elimination of spv_track_constant intrinsic usage for scalar constants, rework of spv_assign_name, removal of the gMIR GET_XXX pseudo code and a smaller number of generated ASSIGN_TYPE pseudo codes substantially decrease volume of data generated during translation.

  2. Simpler code and easier maintenance. The duplicate tracker implementation is simplified, as well as other features.

  3. Numerous fixes of issues and logical flaws in different passes. The main achievement is rework of the duplicate tracker itself that had never guaranteed a correct caching of LLVM IR entities, rarely and randomly returning stale/incorrect records (like, remove an instruction from gMIR but still refer to it). Other fixes comprise consistent generation of OpConstantNull, assigning types to newly created registers, creation of integer/bool types, and other minor fixes.

  4. Numerous fixes of LIT tests: mainly CHECK-DAG to properly reflect SPIR-V spec guarantees, {{$}} at the end of constants to avoid matching of substrings, and XFAILS for SPV_INTEL_long_composites test cases, because the feature is not completed in full yet and doesn't generate a requested by the extension sequence of instructions.

  5. New test cases are added.

Copy link

github-actions bot commented Mar 10, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@VyacheslavLevytskyy VyacheslavLevytskyy changed the title [SPIR-V] Remove spv_track_constant() internal intrinsics [SPIR-V] Rework duplicate tracker and tracking of IR entities and types Mar 14, 2025
@VyacheslavLevytskyy VyacheslavLevytskyy marked this pull request as ready for review March 18, 2025 15:21
@VyacheslavLevytskyy VyacheslavLevytskyy changed the title [SPIR-V] Rework duplicate tracker and tracking of IR entities and types [SPIR-V] Rework duplicate tracker and tracking of IR entities and types to improve compile-time performance Mar 19, 2025
@VyacheslavLevytskyy VyacheslavLevytskyy merged commit 54cc414 into llvm:main Mar 26, 2025
12 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Mar 26, 2025

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-expensive-checks-debian running on gribozavr4 while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/16/builds/16282

Here is the relevant piece of the build log for the reference
Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'LLVM :: CodeGen/SPIRV/structurizer/cf.switch.ifstmt.simple2.ll' FAILED ********************
Exit Code: 2

Command Output (stderr):
--
/b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/llc -mtriple=spirv-unknown-vulkan-compute -O0 /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/CodeGen/SPIRV/structurizer/cf.switch.ifstmt.simple2.ll -o - | /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/FileCheck /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/CodeGen/SPIRV/structurizer/cf.switch.ifstmt.simple2.ll # RUN: at line 1
+ /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/FileCheck /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/CodeGen/SPIRV/structurizer/cf.switch.ifstmt.simple2.ll
+ /b/1/llvm-clang-x86_64-expensive-checks-debian/build/bin/llc -mtriple=spirv-unknown-vulkan-compute -O0 /b/1/llvm-clang-x86_64-expensive-checks-debian/llvm-project/llvm/test/CodeGen/SPIRV/structurizer/cf.switch.ifstmt.simple2.ll -o -

# After SPIRV pre legalizer
# Machine code for function main: IsSSA, TracksLiveness

bb.1.entry:
  successors: %bb.3, %bb.2

  %2:type(s64) = OpTypeFunction %1:type(s64)
  %52:type(s64) = OpTypeInt 32, 0
  %55:type(s64) = OpTypeBool
  %62:type(s64) = OpTypeInt 64, 0
  %1:type(s64) = OpTypeVoid
  %0:iid(s64) = OpFunction %1:type(s64), 2, %2:type(s64)
  OpName %0:iid(s64), 1852399981, 0
  OpEntryPoint 5, %0:iid(s64), 1852399981, 0
  %70:iid(s32) = G_CONSTANT i32 0
  %4:iid(s32) = ASSIGN_TYPE %70:iid(s32), %52:type(s64)
  %69:iid(s1) = G_CONSTANT i1 true
  %6:iid(s1) = ASSIGN_TYPE %69:iid(s1), %55:type(s64)
  %68:iid(s32) = G_CONSTANT i32 5
  %10:iid(s32) = ASSIGN_TYPE %68:iid(s32), %52:type(s64)
  %67:iid(s32) = G_CONSTANT i32 2
  %14:iid(s32) = ASSIGN_TYPE %67:iid(s32), %52:type(s64)
  %66:iid(s32) = G_CONSTANT i32 4
  %20:iid(s32) = ASSIGN_TYPE %66:iid(s32), %52:type(s64)
  %65:iid(s32) = G_CONSTANT i32 6
  %23:iid(s32) = ASSIGN_TYPE %65:iid(s32), %52:type(s64)
  %64:iid(s32) = G_CONSTANT i32 7
  %25:iid(s32) = ASSIGN_TYPE %64:iid(s32), %52:type(s64)
  %63:iid(p0) = G_CONSTANT i64 0
  %27:iid(p0) = ASSIGN_TYPE %63:iid(p0), %62:type(s64)
  %3:iid(s32) = COPY %4:iid(s32)
  OpName %3:iid(s32), 845636978, 544040301, 1869376609, 1881170275, 1953393007, 0
  G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.spv.selection.merge), %bb.27, 0
  G_BRCOND %6:iid(s1), %bb.3
  G_BR %bb.2

bb.2.unreachable:
; predecessors: %bb.1

  G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.spv.unreachable)

...

VyacheslavLevytskyy added a commit that referenced this pull request Mar 28, 2025
…VE_CHECKS enabled (#133142)

After #130605
structurizer/cf.switch.ifstmt.simple2.ll test case starts failing with
the "PHI operand is not live-out from predecessor" diagnostic message.
This test case didn't include usual "-verify-machineinstrs" argument and
the fail was missed before
#130605 merging.

The problem looks not blocking, because the test case successfully
passes its CHECK's. This PR is to fix the build process by mark the test
case as XFAIL when LLVM_ENABLE_EXPENSIVE_CHECKS is enabled.

Investigation of the Machine Verifier complaint is to do. The issue is
created: #133141
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Mar 28, 2025
…BLE_EXPENSIVE_CHECKS enabled (#133142)

After llvm/llvm-project#130605
structurizer/cf.switch.ifstmt.simple2.ll test case starts failing with
the "PHI operand is not live-out from predecessor" diagnostic message.
This test case didn't include usual "-verify-machineinstrs" argument and
the fail was missed before
llvm/llvm-project#130605 merging.

The problem looks not blocking, because the test case successfully
passes its CHECK's. This PR is to fix the build process by mark the test
case as XFAIL when LLVM_ENABLE_EXPENSIVE_CHECKS is enabled.

Investigation of the Machine Verifier complaint is to do. The issue is
created: llvm/llvm-project#133141
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants