This is an archive of the discontinued LLVM Phabricator instance.

[clang][Interp] Implement __builtin_strcmp
ClosedPublic

Authored by tbaeder on May 3 2023, 10:36 PM.

Details

Summary
Make our Function class keep a list of parameter offsets so we can
simply get a parameter by index when evaluating builtin functions.

Diff Detail

Event Timeline

tbaeder created this revision.May 3 2023, 10:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 3 2023, 10:36 PM
tbaeder requested review of this revision.May 3 2023, 10:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 3 2023, 10:37 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
aaron.ballman added inline comments.May 4 2023, 7:54 AM
clang/lib/AST/Interp/Function.h
166–168

Nit: this interface should be taking llvm::SmallVectorImpl<Type> rather than the concrete small vector type.

clang/lib/AST/Interp/InterpBuiltin.cpp
22–23

Thought: it would be nice if we could hoist as much of this implementation as we can into a helper function so that we can share most of the guts with __builtin_memcmp() as well.

Also, it would be good to generalize this so it works for __builtin_wcscmp() and __builtin_wmemcmp() as well.

tbaeder marked an inline comment as done.May 11 2023, 2:48 AM
tbaeder added inline comments.
clang/lib/AST/Interp/InterpBuiltin.cpp
22–23

Definitely, but I think it makes more sense to do that when we implement those functions.

tbaeder updated this revision to Diff 521239.May 11 2023, 2:48 AM
aaron.ballman added inline comments.May 11 2023, 9:28 AM
clang/lib/AST/Interp/InterpBuiltin.cpp
22–23

I guess I was mostly wondering why we don't generalize that now given that we know the need exists. In fact, it seems like you could be handling __builtin_strncmp and others at the same time with one generalized implementation.

tbaeder added inline comments.May 11 2023, 9:30 AM
clang/lib/AST/Interp/InterpBuiltin.cpp
22–23

Because smaller patches get reviewed faster :)

aaron.ballman accepted this revision.May 11 2023, 9:39 AM

LGTM

clang/lib/AST/Interp/InterpBuiltin.cpp
22–23

LoL fair.

This revision is now accepted and ready to land.May 11 2023, 9:39 AM
shafik added inline comments.May 11 2023, 5:34 PM
clang/test/AST/Interp/builtin-functions.cpp
9

Both empty strings "" would be good as well.

tbaeder marked an inline comment as done.May 11 2023, 11:31 PM
This revision was landed with ongoing or failed builds.Jul 20 2023, 6:46 AM
This revision was automatically updated to reflect the committed changes.