Skip to content
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

Apply TBAA metadata in the LLVM backend. #921

Merged
merged 13 commits into from
Nov 2, 2019
Merged

Apply TBAA metadata in the LLVM backend. #921

merged 13 commits into from
Nov 2, 2019

Conversation

nlewycky
Copy link
Contributor

@nlewycky nlewycky commented Oct 30, 2019

Description

Inform LLVM that the pointers to memory, globals, locals, etc., are distinct.

Review

  • Add a short description of the the change to the CHANGELOG.md file

…LVMFunctionCodeGenerator.

Use that to generate distinct TBAA labels for distinct local variables.
…ses off the context.

Move tbaa_label to intrinsics.rs. Move TBAA pass to first in the list, it doesn't get invalidated. Add TBAA labels for internal fields.
Copy link
Contributor

@Hywan Hywan left a comment

Choose a reason for hiding this comment

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

That's a long PR. It looks good to me, but I would not refuse more explanations :-).

@@ -1027,3 +1164,43 @@ impl<'a> CtxType<'a> {
}
}
}

pub fn tbaa_label(
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you document this part specifically please? Ideally, it should explain what TBAA means (is it Type Based Alias Analysis?), what it is used for, and some examples. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added comments both to the function declaration, what it does for the caller, and in its implementation, how it does what it achieves, with the goal that a future developer could follow along with the newly commended code in one hand and the LLVM TBAA documentation in the other.

I didn't add examples. Let me know if you'd like me to extend the comments further.

@Hywan Hywan added 🎉 enhancement New feature! 📦 lib-compiler-llvm About wasmer-compiler-llvm labels Oct 31, 2019
@Hywan
Copy link
Contributor

Hywan commented Oct 31, 2019

Is there a way to test this new feature?

@nlewycky
Copy link
Contributor Author

nlewycky commented Oct 31, 2019

Is there a way to test this new feature?

Don't I wish!!

The difference is only visible to the user in two ways, different generated object code (and with it different size and performance), and different LLVM IR (which in turn generates the different object code). We have some debugging flags --llvm-object-file and --llvm-post-opt-ir and --llvm-pre-opt-ir, but we don't have any tests for those either (and indeed they were broken for a while due to their lack of testing). I've asked @MarkMcCaskey how we should test those flags, but we didn't have any good ideas so I left it. I'd like to start by testing those flags, then we use them to test changes to the LLVM backend.

If we make a change like adding TBAA, that leads to a pre-opt-ir change, changing the pass pipeline to take advantage of TBAA leads to a post-opt-ir change, and object file change. Changing whether we emit PIC code only causes an object file change. Unfortunately, if we simply had golden file tests, making any of these three changes would impact every single test.

I'd welcome any ideas on how to write good tests for this.

Memory can't change between static and dynamic, so use that in the TBAA label
name.

Distinguish between local and imported memory, table and globals.
Copy link
Contributor

@Hywan Hywan left a comment

Choose a reason for hiding this comment

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

Perfect, thank you very much!

@nlewycky
Copy link
Contributor Author

bors r+

bors bot added a commit that referenced this pull request Oct 31, 2019
921: Apply TBAA metadata in the LLVM backend. r=nlewycky a=nlewycky

# Description
Inform LLVM that the pointers to memory, globals, locals, etc., are distinct.

# Review

- [ ] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Nick Lewycky <[email protected]>
Co-authored-by: nlewycky <[email protected]>
@bors
Copy link
Contributor

bors bot commented Oct 31, 2019

Build failed

  • wasmerio.wasmer

@nlewycky
Copy link
Contributor Author

nlewycky commented Nov 2, 2019

bors r+

bors bot added a commit that referenced this pull request Nov 2, 2019
921: Apply TBAA metadata in the LLVM backend. r=nlewycky a=nlewycky

# Description
Inform LLVM that the pointers to memory, globals, locals, etc., are distinct.

# Review

- [x] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Nick Lewycky <[email protected]>
Co-authored-by: nlewycky <[email protected]>
@bors
Copy link
Contributor

bors bot commented Nov 2, 2019

Build succeeded

  • wasmerio.wasmer

@bors bors bot merged commit c0c7661 into master Nov 2, 2019
@bors bors bot deleted the feature/llvm-tbaa branch November 2, 2019 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 enhancement New feature! 📦 lib-compiler-llvm About wasmer-compiler-llvm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants