This is an archive of the discontinued LLVM Phabricator instance.

[clang][Interp] Implement bitcasts (WIP)
AbandonedPublic

Authored by tbaeder on Feb 27 2023, 11:22 PM.

Details

Summary

This implements bitcasts by traversing the pointer fields at interpretation-time, copying them into a buffer and then either creating a new primitive value from the buffer, or traversing the destination pointer and reading the field values from that buffer.

This is a bit non-idiomatic to the new interpreter since we do so much work at interpretation time instead of doing it at compile-time.
I can imagine ways to change this, but it would require bytecode instructions that are rather awkward, I think. So this is my attempt at
doing it this way instead.

It's also missing all the eligibility checks for bitcasts of course.
I have also not tested this on a big-endian host, which I definitely need to do (is the bit rotation needed if the host is BE and the target is BE?).

I'll try to leave a few comments in the diff.

Diff Detail

Event Timeline

tbaeder created this revision.Feb 27 2023, 11:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 27 2023, 11:22 PM
tbaeder requested review of this revision.Feb 27 2023, 11:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 27 2023, 11:22 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
tbaeder edited reviewers, added: shafik; removed: shafi.ahmad.Feb 27 2023, 11:22 PM
tbaeder added inline comments.Feb 27 2023, 11:24 PM
clang/lib/AST/Interp/ByteCodeExprGen.cpp
92

One of the problems here is that right now, all diagnostics are emitted during interpretation time.

clang/lib/AST/Interp/Interp.h
1456

This is a variable sized array. That needs to go of course, but is the best way to heap allocate? Or can we actually use alloca in clang code?

clang/lib/AST/Interp/InterpBuiltin.cpp
33

This needs to be renamed obviously, but I was wondering if this already exists somewhere in the llvm/clang codebase...

81

Same here, I feel like this should already be available?

tbaeder updated this revision to Diff 511386.Apr 6 2023, 6:00 AM
tbaeder updated this revision to Diff 514572.Apr 18 2023, 2:18 AM
tbaeder updated this revision to Diff 515659.Apr 21 2023, 2:32 AM
tbaeder added a comment.EditedMay 11 2023, 2:49 AM

Ping

Pig

🐷

clang/lib/AST/Interp/ByteCodeExprGen.cpp
74

Member pointer types? Unions? volatile-qualified types? There's quite a few restrictions here for constexpr contexts.

87

We returned earlier if ToType is an array, so this comment is a bit suspicious.

92

Didn't we early return in this case as well?

101

All the same restrictions for ToType apply to FromType: http://eel.is/c++draft/bit.cast#3

clang/lib/AST/Interp/Integral.h
179

Consistency with below.

clang/lib/AST/Interp/Interp.h
1456

We usually go with:

std::vector<std::byte> Buff(BuffSize);
if (!DoBitCast(S, FromPtr, Buff.data(), BuffSize))
  return false;
clang/lib/AST/Interp/InterpBuiltin.cpp
33

I'm not aware of one, but perhaps this exists in LLVM somewhere?

35

We should probably be consistent about using size_t in this class so there's no chance of size conversion between types.

81
tbaeder updated this revision to Diff 521574.May 12 2023, 1:01 AM
tbaeder marked 3 inline comments as done.

There's still quite a few unaddressed comments, FWIW.

clang/lib/AST/Interp/Boolean.h
113
aaron.ballman requested changes to this revision.Jun 9 2023, 7:12 AM

Making it clear that there's more work here.

This revision now requires changes to proceed.Jun 9 2023, 7:12 AM

Yes, FWIW I looked into this again last week and noticed a few unsolved problems as well. I'll post a comment once I consider it ready for actual review.

tbaeder abandoned this revision.Jul 11 2023, 2:59 AM

I'm gonna close this, since the new approach is too different.