This is an archive of the discontinued LLVM Phabricator instance.

[clang][Interp] Implement __builtin_offsetof
ClosedPublic

Authored by tbaeder on Jul 27 2023, 12:52 AM.

Details

Summary

Put all the index expressions on the stack and cast them to SInt64. Then add a new OffsetOf opcode so we can evaluate the index expressions.

Diff Detail

Event Timeline

tbaeder created this revision.Jul 27 2023, 12:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 27 2023, 12:52 AM
tbaeder requested review of this revision.Jul 27 2023, 12:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 27 2023, 12:52 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
tbaeder updated this revision to Diff 544693.Jul 27 2023, 3:54 AM
tbaeder updated this revision to Diff 544823.Jul 27 2023, 9:42 AM
cor3ntin added inline comments.Aug 3 2023, 5:04 AM
clang/lib/AST/Interp/ByteCodeExprGen.cpp
1536–1537

Shouldn't that be size_t?

clang/lib/AST/Interp/Interp.h
1901

Might be useful to use a small vector here

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

This function is the same as IntExprEvaluator::VisitOffsetOfExpr - except for the array case. It's frustrating me but I don't have a good solution

cor3ntin added inline comments.Aug 3 2023, 5:19 AM
clang/lib/AST/Interp/Interp.h
1902

can you use size_t here?
It's already a bug nest but it can't hurt to use better types, even if getNumExpressions returns unsigned
https://godbolt.org/z/vhYxose4o

tbaeder added inline comments.Aug 10 2023, 11:28 PM
clang/lib/AST/Interp/ByteCodeExprGen.cpp
1536–1537

The point of the conversion is that we later know the type, i.e. it will be an int64 and we can count on that in OffsetOf(). If we don't do this, we'd have to classify() while interpreting, which is also a possibility.

Generally looks good but still a few unaddressed comments

clang/lib/AST/Interp/ByteCodeExprGen.cpp
1536–1537

I'm happy leaving that as is.

tbaeder updated this revision to Diff 556339.Sep 9 2023, 12:10 AM
tbaeder marked 4 inline comments as done.
cor3ntin accepted this revision.Sep 9 2023, 2:55 AM

LGTM, thanks

This revision is now accepted and ready to land.Sep 9 2023, 2:55 AM
This revision was landed with ongoing or failed builds.Sep 11 2023, 3:04 AM
This revision was automatically updated to reflect the committed changes.
clang/lib/AST/Interp/Opcodes.td