This is an archive of the discontinued LLVM Phabricator instance.

[clang][Interp] __builtin_bit_cast, Take 2
AbandonedPublic

Authored by tbaeder on Jul 11 2023, 4:47 AM.

Details

Summary

This implements __builtin_bit_cast().

The new opcodes are BitCast, which bitcasts to a non-floating primitive type, BitCastFP, which bitcasts to a floating type, and BitCastPtr, which bitcasts to a composite type. Everything is implemented in InterpBitcast.cpp.

Diff Detail

Event Timeline

tbaeder created this revision.Jul 11 2023, 4:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2023, 4:47 AM
tbaeder requested review of this revision.Jul 11 2023, 4:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2023, 4:47 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
MitalAshok added inline comments.
clang/lib/AST/Interp/Boolean.h
113

Does this handle padding bits correctly? E.g., __builtin_bit_cast(bool, (unsigned char) 0b10u) should be false

tbaeder added inline comments.Jul 25 2023, 12:29 PM
clang/lib/AST/Interp/Boolean.h
113

No, it doesn't. But clang has a completely different opinion about that test.

aaron.ballman added inline comments.Aug 3 2023, 10:37 AM
clang/lib/AST/Interp/Boolean.h
113

I think Clang diagnoses that as an invalid constant expression because of https://eel.is/c++draft/utilities#bit.cast-2.sentence-5 which we're allowed to do because of http://eel.is/c++draft/expr.const#5.31

clang/lib/AST/Interp/Interp.h
1546

Should we be checking that we don't have a member pointer at this point?

clang/lib/AST/Interp/InterpBitcast.cpp
19

Alternatively: e-duplicate2000

71

Don't we need to track this at the *bit* level instead of the *byte* level? e.g., padding bits in structures, anonymous bit-fields, bool, _BitInt, etc?

72

llvm::BitVector instead of std::vector<bool>?

tbaeder added inline comments.Aug 4 2023, 3:23 AM
clang/lib/AST/Interp/InterpBitcast.cpp
71

I actually started doing it in bits, but later realized that I can't use that since I have little support for bitfields and the current interpreter doesn't support bitcasts between bitfields. (That's why it's still IndeterminateBits down in BitcastBuffer).

aaron.ballman added inline comments.Aug 4 2023, 4:53 AM
clang/lib/AST/Interp/InterpBitcast.cpp
71

I think we have to do it at the bit level if we want to conform to the standard, so perhaps the best path forward is to work on bitfield support first, then come back to bitcast?

tbaeder abandoned this revision.Oct 10 2023, 12:35 AM

Abandoning this since I've migrated it to Github: https://github.com/llvm/llvm-project/pull/68288