Skip to content

[SandboxIR] Added setVolatile member function to LoadInst and StoreInst #101284

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 6 commits into from
Closed

[SandboxIR] Added setVolatile member function to LoadInst and StoreInst #101284

wants to merge 6 commits into from

Conversation

wizardengineer
Copy link
Contributor

This patch include the implementation of setVolatile() for the LoadInst and StoreInst. Along with test cases.

cc: @vporpo
@wizardengineer wizardengineer changed the title Sandbox ir added set volatile function to load inst and store inst [SandboxIR] Added setVolatile member function to LoadInst and StoreInst Jul 31, 2024
@github-actions GitHub Actions
Copy link

⚠️ 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 GitHub Actions
Copy link

github-actions bot commented Jul 31, 2024 •
edited
Loading

✅ With the latest revision this PR passed the C/C++ code formatter.
vporpo
vporpo reviewed Jul 31, 2024
View reviewed changes
Copy link

@vporpo vporpo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also need to test the SetVolatile tracker object with a test in llvm/unittests/SandboxIR/TrackerTest.cpp. See how it's done in the existing tests.

You start tracking with Ctx.save(), then you will set the volatile value Store->setVolatile(true) then you will check that it was actually set EXPECT_TRUE(Store->isVolatile()), then you will revert the state of the IR with Ctx.revert() and check that the volatile flag was back to its original value EXPECT_FALSE(Store->isVolatile()).

Same for the Load (in the same test).
vporpo
vporpo reviewed Jul 31, 2024
View reviewed changes
vporpo
vporpo reviewed Aug 1, 2024
View reviewed changes
@wizardengineer
Copy link
Contributor Author

Need to close this PR and create a new one to resolve the branch conflicts.
@wizardengineer wizardengineer deleted the _SandboxIR_Added_setVolatile_function_to_LoadInst_and_StoreInst branch August 2, 2024 17:54
@wizardengineer wizardengineer mentioned this pull request Aug 2, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants

@wizardengineer @vporpo