[SandboxIR] Added new StoreInst::create() functions with isVolatile arg #100961

wizardengineer · 2024-07-29T02:34:32Z

Closed PR: #100957

As stated this patch just implements the volatile-memory data for StoreInst

github-actions · 2024-07-29T02:36:17Z

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

github-actions · 2024-07-29T02:38:24Z

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

llvm/include/llvm/SandboxIR/SandboxIR.h

wizardengineer · 2024-07-29T19:15:52Z

@vporpo after the bots pass, is it okay for you to merge this or Is there more changes need to be made?

llvm/include/llvm/SandboxIR/SandboxIR.h

llvm/unittests/SandboxIR/SandboxIRTest.cpp

vporpo

A few minor changes. Looks good.

llvm/unittests/SandboxIR/SandboxIRTest.cpp

llvm/include/llvm/SandboxIR/SandboxIR.h

vporpo

Looks good, thanks for the patch. I will merge it as soon as the bots pass.

vporpo · 2024-07-29T20:24:37Z

Adding setVolatile() is a bit trickier because it requires you to track the change with the Tracker. You would need to create a class in Tracker.h like class SetVolatile : public IRChangeBase containing an Instruction *I and a bool WasVolatile to track the original state of the Volatile flag.
You can check how this is done in a couple of places, like for example: CallBrInst::setDefaultDest(). Take a look at TrackerTest.cpp too.

wizardengineer · 2024-07-29T20:24:53Z

I noticed I can't directly request reviewers for SandboxIR contributions. Would it be alright if I continue mentioning you and other relevant reviewers for my patches?

vporpo · 2024-07-29T20:34:04Z

I noticed I can't directly request reviewers for SandboxIR contributions.

Hmm what do you mean? Doesn't clicking on "Reviewers" and adding my or other people's name work?

wizardengineer · 2024-07-29T21:19:46Z

Hmm what do you mean? Doesn't clicking on "Reviewers" and adding my or other people's name work?

It doesn't seems as if I can. I don't think I have the permissions for that.

vporpo · 2024-07-29T21:28:00Z

Don't worry about it, just mention me and I will review the patch.

llvm-ci · 2024-07-30T00:52:19Z

LLVM Buildbot has detected a new failure on builder sanitizer-x86_64-linux running on sanitizer-buildbot2 while building llvm at step 2 "annotate".

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

Here is the relevant piece of the build log for the reference:

Step 2 (annotate) failure: 'python ../sanitizer_buildbot/sanitizers/zorg/buildbot/builders/sanitizers/buildbot_selector.py' (failure)
...
[374/378] Generating MSAN_INST_TEST_OBJECTS.msan_test.cpp.x86_64-with-call.o
[375/378] Generating Msan-x86_64-with-call-Test
[376/378] Generating MSAN_INST_TEST_OBJECTS.msan_test.cpp.x86_64.o
[377/378] Generating Msan-x86_64-Test
[377/378] Running compiler_rt regression tests
llvm-lit: /b/sanitizer-x86_64-linux/build/llvm-project/llvm/utils/lit/lit/discovery.py:276: warning: input '/b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/rtsan/X86_64LinuxConfig' contained no tests
llvm-lit: /b/sanitizer-x86_64-linux/build/llvm-project/llvm/utils/lit/lit/main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 900 seconds was requested on the command line. Forcing timeout to be 900 seconds.
-- Testing: 10076 tests, 88 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
TIMEOUT: SanitizerCommon-hwasan-x86_64-Linux :: Posix/fork_threaded.c (10076 of 10076)
******************** TEST 'SanitizerCommon-hwasan-x86_64-Linux :: Posix/fork_threaded.c' FAILED ********************
Exit Code: -9
Timeout: Reached timeout of 900 seconds

Command Output (stderr):
--
RUN: at line 1: /b/sanitizer-x86_64-linux/build/build_default/./bin/clang  -gline-tables-only -fsanitize=hwaddress -fuse-ld=lld -fsanitize-hwaddress-experimental-aliasing  -m64 -funwind-tables  -I/b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test -ldl -O0 /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Posix/fork_threaded.c -o /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/hwasan-x86_64-Linux/Posix/Output/fork_threaded.c.tmp && env HWASAN_OPTIONS=die_after_fork=0  /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/hwasan-x86_64-Linux/Posix/Output/fork_threaded.c.tmp
+ /b/sanitizer-x86_64-linux/build/build_default/./bin/clang -gline-tables-only -fsanitize=hwaddress -fuse-ld=lld -fsanitize-hwaddress-experimental-aliasing -m64 -funwind-tables -I/b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test -ldl -O0 /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Posix/fork_threaded.c -o /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/hwasan-x86_64-Linux/Posix/Output/fork_threaded.c.tmp
+ env HWASAN_OPTIONS=die_after_fork=0 /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/hwasan-x86_64-Linux/Posix/Output/fork_threaded.c.tmp
==1125000==ERROR: HWAddressSanitizer: tag-mismatch on address 0x481400005140 at pc 0x5c8144e15e37
READ of size 155 at 0x481400005140 tags: 02/00 (ptr/mem) in thread T0

=================================================================
==1124307==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 568 byte(s) in 3 object(s) allocated from:
    #0 0x5c8144e0a548 in malloc /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/lib/hwasan/hwasan_allocation_functions.cpp:151:3
    #1 0x5c8144f627ee in operator new(unsigned long) llvm-link
    #2 0x5c8144e009f4 in _start (/b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/hwasan-x86_64-Linux/Posix/Output/fork_threaded.c.tmp+0xee9f4)

Direct leak of 204 byte(s) in 1 object(s) allocated from:
    #0 0x5c8144e0a08d in calloc /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/lib/hwasan/hwasan_allocation_functions.cpp:127:3
    #1 0x5c8144eeb0f9 in llvm::safe_calloc(unsigned long, unsigned long) llvm-link
    #2 0x5c8144e009f4 in _start (/b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/hwasan-x86_64-Linux/Posix/Output/fork_threaded.c.tmp+0xee9f4)

Indirect leak of 1752 byte(s) in 42 object(s) allocated from:
    #0 0x5c8144e09a49 in aligned_alloc /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/lib/hwasan/hwasan_allocation_functions.cpp:58:3
    #1 0x5c8144ef89e7 in llvm::allocate_buffer(unsigned long, unsigned long) llvm-link

Indirect leak of 780 byte(s) in 1 object(s) allocated from:
    #0 0x5c8144e0a08d in calloc /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/lib/hwasan/hwasan_allocation_functions.cpp:127:3
    #1 0x5c8144eeb0f9 in llvm::safe_calloc(unsigned long, unsigned long) llvm-link

Indirect leak of 160 byte(s) in 1 object(s) allocated from:
    #0 0x5c8144e0a548 in malloc /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/lib/hwasan/hwasan_allocation_functions.cpp:151:3
    #1 0x5c8144f627ee in operator new(unsigned long) llvm-link
    #2 0x5c8144e009f4 in _start (/b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/hwasan-x86_64-Linux/Posix/Output/fork_threaded.c.tmp+0xee9f4)

SUMMARY: HWAddressSanitizer: 3464 byte(s) leaked in 48 allocation(s).
Step 9 (test compiler-rt symbolizer) failure: test compiler-rt symbolizer (failure)
...
[374/378] Generating MSAN_INST_TEST_OBJECTS.msan_test.cpp.x86_64-with-call.o
[375/378] Generating Msan-x86_64-with-call-Test
[376/378] Generating MSAN_INST_TEST_OBJECTS.msan_test.cpp.x86_64.o
[377/378] Generating Msan-x86_64-Test
[377/378] Running compiler_rt regression tests
llvm-lit: /b/sanitizer-x86_64-linux/build/llvm-project/llvm/utils/lit/lit/discovery.py:276: warning: input '/b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/rtsan/X86_64LinuxConfig' contained no tests
llvm-lit: /b/sanitizer-x86_64-linux/build/llvm-project/llvm/utils/lit/lit/main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 900 seconds was requested on the command line. Forcing timeout to be 900 seconds.
-- Testing: 10076 tests, 88 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
TIMEOUT: SanitizerCommon-hwasan-x86_64-Linux :: Posix/fork_threaded.c (10076 of 10076)
******************** TEST 'SanitizerCommon-hwasan-x86_64-Linux :: Posix/fork_threaded.c' FAILED ********************
Exit Code: -9
Timeout: Reached timeout of 900 seconds

Command Output (stderr):
--
RUN: at line 1: /b/sanitizer-x86_64-linux/build/build_default/./bin/clang  -gline-tables-only -fsanitize=hwaddress -fuse-ld=lld -fsanitize-hwaddress-experimental-aliasing  -m64 -funwind-tables  -I/b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test -ldl -O0 /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Posix/fork_threaded.c -o /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/hwasan-x86_64-Linux/Posix/Output/fork_threaded.c.tmp && env HWASAN_OPTIONS=die_after_fork=0  /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/hwasan-x86_64-Linux/Posix/Output/fork_threaded.c.tmp
+ /b/sanitizer-x86_64-linux/build/build_default/./bin/clang -gline-tables-only -fsanitize=hwaddress -fuse-ld=lld -fsanitize-hwaddress-experimental-aliasing -m64 -funwind-tables -I/b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test -ldl -O0 /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Posix/fork_threaded.c -o /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/hwasan-x86_64-Linux/Posix/Output/fork_threaded.c.tmp
+ env HWASAN_OPTIONS=die_after_fork=0 /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/hwasan-x86_64-Linux/Posix/Output/fork_threaded.c.tmp
==1125000==ERROR: HWAddressSanitizer: tag-mismatch on address 0x481400005140 at pc 0x5c8144e15e37
READ of size 155 at 0x481400005140 tags: 02/00 (ptr/mem) in thread T0

=================================================================
==1124307==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 568 byte(s) in 3 object(s) allocated from:
    #0 0x5c8144e0a548 in malloc /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/lib/hwasan/hwasan_allocation_functions.cpp:151:3
    #1 0x5c8144f627ee in operator new(unsigned long) llvm-link
    #2 0x5c8144e009f4 in _start (/b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/hwasan-x86_64-Linux/Posix/Output/fork_threaded.c.tmp+0xee9f4)

Direct leak of 204 byte(s) in 1 object(s) allocated from:
    #0 0x5c8144e0a08d in calloc /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/lib/hwasan/hwasan_allocation_functions.cpp:127:3
    #1 0x5c8144eeb0f9 in llvm::safe_calloc(unsigned long, unsigned long) llvm-link
    #2 0x5c8144e009f4 in _start (/b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/hwasan-x86_64-Linux/Posix/Output/fork_threaded.c.tmp+0xee9f4)

Indirect leak of 1752 byte(s) in 42 object(s) allocated from:
    #0 0x5c8144e09a49 in aligned_alloc /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/lib/hwasan/hwasan_allocation_functions.cpp:58:3
    #1 0x5c8144ef89e7 in llvm::allocate_buffer(unsigned long, unsigned long) llvm-link

Indirect leak of 780 byte(s) in 1 object(s) allocated from:
    #0 0x5c8144e0a08d in calloc /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/lib/hwasan/hwasan_allocation_functions.cpp:127:3
    #1 0x5c8144eeb0f9 in llvm::safe_calloc(unsigned long, unsigned long) llvm-link

Indirect leak of 160 byte(s) in 1 object(s) allocated from:
    #0 0x5c8144e0a548 in malloc /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/lib/hwasan/hwasan_allocation_functions.cpp:151:3
    #1 0x5c8144f627ee in operator new(unsigned long) llvm-link
    #2 0x5c8144e009f4 in _start (/b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/hwasan-x86_64-Linux/Posix/Output/fork_threaded.c.tmp+0xee9f4)

SUMMARY: HWAddressSanitizer: 3464 byte(s) leaked in 48 allocation(s).

wizardengineer · 2024-07-30T06:46:24Z

Adding setVerbose() is a bit trickier because it requires you to track the change with the Tracker. You would need to create a class in Tracker.h like class SetVerbose : public IRChangeBase containing an Instruction *I and a bool WasVerbose to track the original state of the Verbose flag.
You can check how this is done in a couple of places, like for example: CallBrInst::setDefaultDest(). Take a look at TrackerTest.cpp too.

@vporpo Just to be clear, so I'm implementing the setVerbose() method for the StoreInst or is it something completely separate?

vporpo · 2024-07-30T15:14:46Z

so I'm implementing the setVerbose() method for the StoreInst or is it something completely separate?

Yes, both llvm::LoadInst and llvm::StoreInst have a setVolatile(bool) function, which is missing from both sandboxir::LoadInst and sandboxir::StoreInst.

vporpo · 2024-07-30T17:32:34Z

@medievalghoul sorry I misspelled: I meant setVolatile(bool). I updated the comments above too. Sorry about that.

wizardengineer · 2024-07-30T19:33:38Z

No worries, thanks for letting me know.

[SandboxIR] Added new StoreInst::create() functions with isVolatile arg

3a96061

This comment was marked as resolved.

Sign in to view

fixed formatting

fd43984

This comment was marked as resolved.

Sign in to view

simple code refactoring

6231816

wizardengineer mentioned this pull request Jul 29, 2024

[SandboxIR][NFC] Fixes for LoadInst::create functions #100955

Merged

vporpo reviewed Jul 29, 2024

View reviewed changes

llvm/include/llvm/SandboxIR/SandboxIR.h Outdated Show resolved Hide resolved

wizardengineer added 2 commits July 29, 2024 14:04

minor change: fixed StoreInst

a958fd2

added more functional tests for StoreInst

8e52ce3

wizardengineer requested a review from vporpo July 29, 2024 19:17

vporpo reviewed Jul 29, 2024

View reviewed changes

llvm/include/llvm/SandboxIR/SandboxIR.h Outdated Show resolved Hide resolved

llvm/unittests/SandboxIR/SandboxIRTest.cpp Outdated Show resolved Hide resolved

llvm/unittests/SandboxIR/SandboxIRTest.cpp Outdated Show resolved Hide resolved

wizardengineer added 2 commits July 29, 2024 15:39

fixed some comments

ffda443

added check the instruction position

a7c893e

vporpo reviewed Jul 29, 2024

View reviewed changes

simple fixes related to format and unit testing

fd64c9b

vporpo approved these changes Jul 29, 2024

View reviewed changes

wizardengineer requested a review from vporpo July 29, 2024 23:18

vporpo merged commit 42326c7 into llvm:main Jul 29, 2024
7 checks passed

wizardengineer mentioned this pull request Jul 29, 2024

[SandboxIR][NFC] Removed comments from LoadInst test case #101099

Merged

wizardengineer deleted the _SandboxIR_Added_new_StoreInst_create_functions_with_isVolatile_arg branch August 2, 2024 17:55

[SandboxIR] Added new StoreInst::create() functions with isVolatile arg #100961

[SandboxIR] Added new StoreInst::create() functions with isVolatile arg #100961

Uh oh!

Conversation

wizardengineer commented Jul 29, 2024

Uh oh!

github-actions bot commented Jul 29, 2024

Uh oh!

github-actions bot commented Jul 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

Uh oh!

wizardengineer commented Jul 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vporpo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vporpo left a comment

Choose a reason for hiding this comment

Uh oh!

vporpo commented Jul 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wizardengineer commented Jul 29, 2024

Uh oh!

vporpo commented Jul 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wizardengineer commented Jul 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vporpo commented Jul 29, 2024

Uh oh!

Uh oh!

llvm-ci commented Jul 30, 2024

Uh oh!

wizardengineer commented Jul 30, 2024

Uh oh!

vporpo commented Jul 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vporpo commented Jul 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wizardengineer commented Jul 30, 2024

Uh oh!

Uh oh!

github-actions bot commented Jul 29, 2024 •

edited

Loading

wizardengineer commented Jul 29, 2024 •

edited

Loading

vporpo commented Jul 29, 2024 •

edited

Loading

vporpo commented Jul 29, 2024 •

edited

Loading

wizardengineer commented Jul 29, 2024 •

edited

Loading

vporpo commented Jul 30, 2024 •

edited

Loading

vporpo commented Jul 30, 2024 •

edited

Loading