[llvm-commits] Review Request: Use SmallPtrSetImpl instead of SmallPtrSet in funciton IVUsers::AddUsersIfInteresting

hi,

Yep. I normally do that. I was under some strange impression last night that SmallPtrSetImpl wasn't a template.

The patch is incorrect because the SmallPtrSetImpl is neither a
template nor has an "insert" function...

After a detailed look at the header of SmallPtrSet, I found that the
SmallPtrSetImpl has an "insert_imp" function which accepts void
pointer as argument, and the "insert" function in SmallPtrSet simply
cast the incoming pointer to void* by the
"PtrTraits::getAsVoidPointer" function and pass the void pointer to
insert_imp.

So I wonder can we make SmallPtrSetImpl become a template just like
SmallVectorImpl? After that we can move insert/erase/count from
SmallPtrSet to SmallPtrSetImpl, and users can pass a
SmallPtrSetImpl<T> as argument instead of pass something like
SmallPtrSet<T, 100>.

Please check in, and you can simultaneously fix the polly branch.

Temporary fixed by passing a dummy set.

Incidentally, the only reason I didn't hide the SmallPtrSet argument behind the API is that all the callers outside IVUsers will be removed from the codebase soon.

So which function is supposed to call by outside caller?

-Andy

best regards
ether

hi,

Yep. I normally do that. I was under some strange impression last night that SmallPtrSetImpl wasn't a template.

The patch is incorrect because the SmallPtrSetImpl is neither a
template nor has an "insert" function...

After a detailed look at the header of SmallPtrSet, I found that the
SmallPtrSetImpl has an "insert_imp" function which accepts void
pointer as argument, and the "insert" function in SmallPtrSet simply
cast the incoming pointer to void* by the
"PtrTraits::getAsVoidPointer" function and pass the void pointer to
insert_imp.

So I wonder can we make SmallPtrSetImpl become a template just like
SmallVectorImpl? After that we can move insert/erase/count from
SmallPtrSet to SmallPtrSetImpl, and users can pass a
SmallPtrSetImpl<T> as argument instead of pass something like
SmallPtrSet<T, 100>.

I would personally like that. I think it was done this way to avoid code bloat. But as long as you keep the void* implementation in a nontemplate class (SmallPtrSetBase?), then I don't see a problem. You would effectively be moving some template instantiations from SmallPtrSet<T,size> into SmallPtrSetImpl<T>, which could be an improvement.

Please check in, and you can simultaneously fix the polly branch.

Temporary fixed by passing a dummy set.

Incidentally, the only reason I didn't hide the SmallPtrSet argument behind the API is that all the callers outside IVUsers will be removed from the codebase soon.

So which function is supposed to call by outside caller?

AddUsersIfInteresting is the correct function to call to update IVUsers. But soon LSR will be the only client of IVUsers, and IndVarSimplify won't need to update IVUsers at all.

-Andy