Conversation
| return origins; | ||
| } | ||
|
|
||
| // Given an origin value, return the possible candidate pointee type for ptr. |
There was a problem hiding this comment.
please move new type inference from DsaLocal.cc file so that it is easier to see what changed and what is old.
Create 2 PRs. One PR with the original adaptation for LLVM15, and a separate PR for type inference. The type inference PR should be relative to the first PR so that it only includes the new type inference stuff.
This will be much easier to review
|
|
||
| // Given a pointer value, yield all instructions/values that define or use it. | ||
| static llvm::SmallVector<const Value *, 8> | ||
| findPointerOrigins(const Value *ptr) { |
There was a problem hiding this comment.
this seems like a very strange function. It converts an iterator into a vector.
This is usually not needed. If needed, there are simple one-line ways to do it No need for a function.
There was a problem hiding this comment.
Besides converting the iterator to a vector, it also pushes in the origin of the pointer. If this doesn't warrant a function, I'll think of another way to iterate the origin and users.
There was a problem hiding this comment.
chatgpt suggests the following improvement
llvm::SmallVector<const llvm::Value *, 8> findPointerOrigins(const llvm::Value *ptr) {
llvm::SmallVector<const llvm::Value *, 8> origins;
origins.push_back(ptr);
llvm::append_range(origins, ptr->users());
return origins;
}```
There was a problem hiding this comment.
however, with this is, it is a bit strange that the pointer and its uses are all placed into one vector. It is not clear how the vector will be processed later. If the vector is a todo list, then it would make more sense to allocate it outside of the function.
| } | ||
|
|
||
| // Given an origin value, return the possible candidate pointee type for ptr. | ||
| // - If analysis is inconclusive or ambiguous, return nullptr |
There was a problem hiding this comment.
comment says 'return nullptr', but the function does not return a ptr
There was a problem hiding this comment.
Fixing for the next push. Forgot to remove the comment after API change.
|
|
||
| // Given an origin value, return the possible candidate pointee type for ptr. | ||
| // - If analysis is inconclusive or ambiguous, return nullptr | ||
| static SmallPtrSet<Type *, 2> getPointeeTypesFromOrigin(const Value *ptr, |
There was a problem hiding this comment.
please write a more detailed spec for this function. I can't tell what it supposed to do exactly from the source. Makes hard to comment on whether it is appropriate.
2 participants
This branch is a draft.
Let me know if I'm on the right track.
Currently the feature is very barebone but will try to be test- and result-driven, so I'm not expanding its functionalities until there's sufficient evidence it is recovering precision. Currently repairing the test suite for LLVM 15 to allow this.