This is an archive of the discontinued LLVM Phabricator instance.

[KnownBits] Add methods to set and clear all bits.
ClosedPublic

Authored by craig.topper on Apr 28 2017, 12:16 AM.

Details

Summary

This adds a clearAll and setAll method to KnownBits that call either clearAllBits or setAllBits on both.

We also still need methods to clear Zero/One and and set all bits in the opposite. I'm considering makeZero() and makeAllOnes(). As I'm writing this I'm wondering if clearAllBits should be makeUnknown. Not sure what setAll would be in that naming.

I'm worried about mixing the "clearing" a bit to unknown state vs making a bit known to be zero which would be consider clearing its value. I want to be able to treat KnownBits as a value so we can say is it known negative or nonnegative or zero or all ones. But I also want to be able to refer to its unknown/known set of bits.

Diff Detail

Event Timeline

craig.topper created this revision.Apr 28 2017, 12:16 AM

I think my previous diff may have been incomplete.

Fix a misnamed call.

RKSimon edited edge metadata.Apr 28 2017, 1:32 AM

This looks incomplete - the addition of the method to the KnownBits struct is missing and there is no context to the diff.

We also still need methods to clear Zero/One and and set all bits in the opposite. I'm considering makeZero() and makeAllOnes(). As I'm writing this I'm wondering if clearAllBits should be makeUnknown. Not sure what setAll would be in that naming.

Would KnowBits::makeInvalid make sense?

Why not KnownBits::clearAllBits() + KnownBits::setAllBits() ?

I've removed the method that sets all bits in both zero and one for now. I think that should be done in a future patch that adds intersecting support since setting all vectors always proceeds an intersection loop. Maybe the method should just be prepareForIntersection or something that stresses that its only useful for that.

I've added methods for testing and setting all ones and all zeros. To preserve existing code semantics I've added two versions of setting zero/one, one that just asserts if it creates a conflict, and one that erases all previous information. The latter is called 'force', but I'm not sure if that's a good name.

Making the vector unknown is now called resetAll. And there is a method to check if the known bits are all unknown.

Let me know if these names make sense. I'm open to other suggestions.

I'm not sure we have a need for both the 'set' and 'force' versions - AFAICT all the 'set' cases above could be 'force' as we're just making use of the fact that we know that the KnownBits are all zero at initialization. Maybe just have 'force' version and call them 'set'? If that make sense at all......

Merge setAllZero and forceAllZero. Same for AllOnes.

RKSimon accepted this revision.May 5 2017, 10:33 AM

LGTM

This revision is now accepted and ready to land.May 5 2017, 10:33 AM
This revision was automatically updated to reflect the committed changes.