Skip to content

Conversation

@DataCorrupted
Copy link
Member

No description provided.

@DataCorrupted DataCorrupted self-assigned this Nov 5, 2024
@DataCorrupted
Copy link
Member Author

@ellishg @kyulee-com @plotfi Did you know why we didn't do this earlier? Is it because there's some issue with isKnownNonZero or simply because we forgot?

This patch is necessary if we want to erase some nil check thunk.

@kyulee-com
Copy link
Contributor

kyulee-com commented Nov 5, 2024

To me, using isKnownNonZero seems reasonable.
Although logically it's same, I slightly prefer this way:

  if (llvm::isKnownNonZero(receiver, CGM.getDataLayout())
    return false;
  // Otherwise, assume it can be null.
  return true;

@ellishg
Copy link
Contributor

ellishg commented Nov 5, 2024

Yeah it seems good to me, but we should have a test case too

@DataCorrupted
Copy link
Member Author

DataCorrupted commented Nov 5, 2024 via email

@DataCorrupted
Copy link
Member Author

The quest to write some tests has failed.

I realized that this call has little to no effect. isKnownNonZero requires some function analysis (e.g. Dominator Tree), which doesn't exist yet during frontend CodeGen.

@DataCorrupted DataCorrupted deleted the NonNull branch February 6, 2025 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants