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

Extra assertions in RS4GC #71201

Merged
merged 5 commits into from
Nov 23, 2023
Merged

Extra assertions in RS4GC #71201

merged 5 commits into from
Nov 23, 2023

Conversation

zduka
Copy link
Contributor

@zduka zduka commented Nov 3, 2023

Adds assertion that the base/derived pointers are of the same size.

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 3, 2023

@llvm/pr-subscribers-llvm-transforms

Author: Petr Maj (zduka)

Changes

Adds assertion that the base/derived pointers are of the same size.


Full diff: https://github.com/llvm/llvm-project/pull/71201.diff

1 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp (+7)
diff --git a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
index 63ee11295e9c032..35945a8fb577545 100644
--- a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
+++ b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
@@ -1249,6 +1249,8 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &Cache,
 
 #ifndef NDEBUG
   VerifyStates();
+  // get the data layout to compare the sizes of base/derived pointer values
+  auto &DL = cast<llvm::Instruction>(Def)->getModule()->getDataLayout();
 #endif
 
   // Cache all of our results so we can cheaply reuse them
@@ -1258,6 +1260,11 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &Cache,
     auto *BDV = Pair.first;
     Value *Base = Pair.second.getBaseValue();
     assert(BDV && Base);
+    // the assumption is that whenever we have a derived ptr(s), their base
+    // ptr(s) must be of the same size, not necessarily the same type
+    assert(DL.getTypeAllocSize(BDV->getType()) ==
+               DL.getTypeAllocSize(Base->getType()) &&
+           "Derived and base values should have same size");
     // Only values that do not have known bases or those that have differing
     // type (scalar versus vector) from a possible known base should be in the
     // lattice.

Copy link

github-actions bot commented Nov 9, 2023

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

Copy link
Contributor

@annamthomas annamthomas left a comment

Choose a reason for hiding this comment

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

LGTM w/ nit.

llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp Outdated Show resolved Hide resolved
Co-authored-by: annamthomas <anna@azul.com>
@zduka
Copy link
Contributor Author

zduka commented Nov 23, 2023

Thank you!

@annamthomas annamthomas merged commit c308d90 into llvm:main Nov 23, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants