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

[SR-8051] FixedTypeInfo::getSpareBitExtraInhabitantCount() is broken #50584

Closed
davezarzycki opened this issue Jun 20, 2018 · 3 comments
Closed
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself IRGen LLVM IR generation

Comments

@davezarzycki
Copy link
Collaborator

Previous ID SR-8051
Radar None
Original Reporter @davezarzycki
Type Bug
Status Closed
Resolution Invalid
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug, IRGen
Assignee None
Priority Medium

md5: cec19f0c0620dadabe28263b8449e140

Issue Description:

FixedTypeInfo::getSpareBitExtraInhabitantCount() is broken. You can see an example of the brokenness in test/IRGen/enum.sil, where the following type exists:

enum SinglePayloadSpareBit {                                                     
  case x(Builtin.Int63)                                                          
  case y                                                                         
  case z                                                                         
} 

The size of the above type should be 65 bits, but the current compiler wrongly thinks that 64 bits are good enough. The fix is as follows:

diff --git i/lib/IRGen/GenType.cpp w/lib/IRGen/GenType.cpp
index ffdadb8fca..0954d60227 100644
--- i/lib/IRGen/GenType.cpp
+++ w/lib/IRGen/GenType.cpp
@@ -239,13 +239,10 @@ unsigned FixedTypeInfo::getSpareBitExtraInhabitantCount() const {
     return 0;
   // The runtime supports a max of 0x7FFFFFFF extra inhabitants, which ought
   // to be enough for anybody.
-  if (getFixedSize().getValue() >= 4)
-    return 0x7FFFFFFF;
-  unsigned spareBitCount = SpareBits.count();
+  auto spareBitCount = std::min(SpareBits.count(), 31ul);
   assert(spareBitCount <= getFixedSize().getValueInBits()
          && "more spare bits than storage bits?!");
-  unsigned inhabitedBitCount = getFixedSize().getValueInBits() - spareBitCount;
-  return ((1U << spareBitCount) - 1U) << inhabitedBitCount;
+  return ((1U << spareBitCount) - 1U);
 }

I would have created a pull request, but I got bogged down in updating the test file and I wasn't sure if people would prefer that the 'z' case be removed at the same time as one deals with the fallout from the code change. Thoughts? This isn't urgent for me, so if somebody wants to power through the minor test suite fallout, I'd really appreciate it.

@rjmccall
Copy link
Member

Having even 1 spare bit should allow that to be used as a discriminator, and then all the other bits in the value can be used for extra inhabitants.

@davezarzycki
Copy link
Collaborator Author

Oh? Great! Never mind then. My mistake.

@rjmccall
Copy link
Member

If you want to audit the code for that, that would be great, of course. 🙂

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself IRGen LLVM IR generation
Projects
None yet
Development

No branches or pull requests

2 participants