Skip to content

[SandboxIR] Implement SandboxIR Type #106294

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
merged 1 commit into from
Aug 30, 2024
Merged

[SandboxIR] Implement SandboxIR Type #106294

merged 1 commit into from
Aug 30, 2024

Conversation

vporpo
Copy link
Contributor

@vporpo vporpo commented Aug 27, 2024

This patch implements sandboxir::Type, a thin wrapper of llvm::Type. This is designed very similarly to sandbox::Value. Context owns all sandboxir::Type objects and maintains a map between llvm::Type and sandboxir::Type.

There are a couple of reasons for migrating from llvm::Type to sandboxir::Type:

  • Creating an llvm::Type from within SandboxIR-only code doesn't work well because it requires you to pass llvm::Context to functions like llvm::Type::getInt32Ty(C), but you wouldn't normally have access to llvm::Context C. In unit tests this is not such a big deal because you have access to both, but it will become an issue in SandboxIR-only code.
  • Not being able to get the sandboxir::Context from llvm::Type results in awkward sandboir APIs with additional sandboxir::Context arguments.
  • llvm::Type::getContext() can basically give you access to the whole LLVM IR, which we should try to avoid.

@aeubanks
Copy link
Contributor

I thought we were going to just use llvm::Type. Can you add in the description why we need this?

@vporpo
Copy link
Contributor Author

vporpo commented Aug 28, 2024

I thought we were going to just use llvm::Type. Can you add in the description why we need this?

There are a couple of reasons:

  • Creating an llvm::Type from within SandboxIR-only code doesn't work well because it requires you to pass llvm::Context to functions like llvm::Type::getInt32Ty(C), but you wouldn't normally have access to llvm::Context C. In unit tests this is not such a big deal because you have access to both, but it will become an issue in SandboxIR-only code.
  • Not being able to get the sandboxir::Context from llvm::Type results in awkward sandboir APIs with additional sandboxir::Context arguments.
  • llvm::Type::getContext() can basically give you access to the whole LLVM IR, which we should try to avoid.

@aeubanks
Copy link
Contributor

makes sense at a high level

@vporpo
Copy link
Contributor Author

vporpo commented Aug 29, 2024

Pushed version includes fixes for the newly landed ExtractValueInst.

This patch implements sandboxir::Type, a thin wrapper of llvm::Type.
This is designed very similarly to sandbox::Value.
Context owns all sandboxir::Type objects and maintains a map between
llvm::Type and sandboxir::Type.

There are a couple of reasons for migrating from llvm::Type to sandboxir::Type:

- Creating an llvm::Type from within SandboxIR-only code doesn't work well
because it requires you to pass llvm::Context to functions like
llvm::Type::getInt32Ty(C), but you wouldn't normally have access to
llvm::Context C.
- Not being able to get the sandboxir::Context from llvm::Type results in
awkward sandboir APIs with additional sandboxir::Context arguments.
- llvm::Type::getContext() can basically give you access to the whole LLVM IR,
which we should try to avoid.
@vporpo vporpo merged commit 034f2b3 into llvm:main Aug 30, 2024
6 of 7 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Aug 30, 2024

LLVM Buildbot has detected a new failure on builder bolt-x86_64-ubuntu-dylib running on bolt-worker while building llvm at step 6 "test-build-bolt-check-bolt".

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

Here is the relevant piece of the build log for the reference
Step 6 (test-build-bolt-check-bolt) failure: test (failure)
******************** TEST 'BOLT :: perf2bolt/perf_test.test' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 5: /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/bin/clang /home/worker/bolt-worker2/llvm-project/bolt/test/perf2bolt/Inputs/perf_test.c -fuse-ld=lld -Wl,--script=/home/worker/bolt-worker2/llvm-project/bolt/test/perf2bolt/Inputs/perf_test.lds -o /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp
+ /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/bin/clang /home/worker/bolt-worker2/llvm-project/bolt/test/perf2bolt/Inputs/perf_test.c -fuse-ld=lld -Wl,--script=/home/worker/bolt-worker2/llvm-project/bolt/test/perf2bolt/Inputs/perf_test.lds -o /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp
RUN: at line 6: perf record -e cycles:u -o /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp2 -- /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp
+ perf record -e cycles:u -o /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp2 -- /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp
Lowering default frequency rate from 4000 to 2000.
Please consider tweaking /proc/sys/kernel/perf_event_max_sample_rate.
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.002 MB /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp2 (18 samples) ]
RUN: at line 7: /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/bin/perf2bolt /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp -p=/home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp2 -o /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp3 -nl -ignore-build-id 2>&1 | /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/bin/FileCheck /home/worker/bolt-worker2/llvm-project/bolt/test/perf2bolt/perf_test.test
+ /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/bin/perf2bolt /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp -p=/home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp2 -o /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp3 -nl -ignore-build-id
+ /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/bin/FileCheck /home/worker/bolt-worker2/llvm-project/bolt/test/perf2bolt/perf_test.test
RUN: at line 12: /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/bin/clang /home/worker/bolt-worker2/llvm-project/bolt/test/perf2bolt/Inputs/perf_test.c -no-pie -fuse-ld=lld -o /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp4
+ /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/bin/clang /home/worker/bolt-worker2/llvm-project/bolt/test/perf2bolt/Inputs/perf_test.c -no-pie -fuse-ld=lld -o /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp4
RUN: at line 13: perf record -e cycles:u -o /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp5 -- /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp4
+ perf record -e cycles:u -o /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp5 -- /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp4
Lowering default frequency rate from 4000 to 2000.
Please consider tweaking /proc/sys/kernel/perf_event_max_sample_rate.
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.002 MB /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp5 (10 samples) ]
RUN: at line 14: /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/bin/perf2bolt /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp4 -p=/home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp5 -o /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp6 -nl -ignore-build-id 2>&1 | /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/bin/FileCheck /home/worker/bolt-worker2/llvm-project/bolt/test/perf2bolt/perf_test.test --check-prefix=CHECK-NO-PIE
+ /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/bin/perf2bolt /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp4 -p=/home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp5 -o /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/tools/bolt/test/perf2bolt/Output/perf_test.test.tmp6 -nl -ignore-build-id
+ /home/worker/bolt-worker2/bolt-x86_64-ubuntu-dylib/build/bin/FileCheck /home/worker/bolt-worker2/llvm-project/bolt/test/perf2bolt/perf_test.test --check-prefix=CHECK-NO-PIE
/home/worker/bolt-worker2/llvm-project/bolt/test/perf2bolt/perf_test.test:17:19: error: CHECK-NO-PIE-NOT: excluded string found in input
CHECK-NO-PIE-NOT: !! WARNING !! This high mismatch ratio indicates the input binary is probably not the same binary used during profiling collection.
                  ^
<stdin>:25:2: note: found here
 !! WARNING !! This high mismatch ratio indicates the input binary is probably not the same binary used during profiling collection. The generated data may be ineffective for improving performance.
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Input file: <stdin>
Check file: /home/worker/bolt-worker2/llvm-project/bolt/test/perf2bolt/perf_test.test

-dump-input=help explains the following input dump.

Input was:
<<<<<<
        .
        .
        .
       20: PERF2BOLT: waiting for perf mem events collection to finish... 
       21: PERF2BOLT: processing basic events (without LBR)... 
       22: PERF2BOLT: read 10 samples 
       23: PERF2BOLT: out of range samples recorded in unknown regions: 9 (90.0%) 
       24:  
       25:  !! WARNING !! This high mismatch ratio indicates the input binary is probably not the same binary used during profiling collection. The generated data may be ineffective for improving performance. 
...

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.

4 participants