Page MenuHomePhabricator

IRGen: Emit an inline implementation of __builtin_wmemcmp on MSVCRT platforms.
ClosedPublic

Authored by pcc on Jan 23 2018, 1:13 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

pcc created this revision.Jan 23 2018, 1:13 PM
rnk added a comment.Jan 23 2018, 1:27 PM

Doesn't this give the wrong ordering for little-endian machines? Won't we say the code point 0xFF00 comes before 0x00FF because it's encoded 0x00, 0xFF vs 0xFF, 0x00?

pcc planned changes to this revision.Jan 23 2018, 1:43 PM

Oops, it does indeed. I guess we'll need to emit an inline implementation.

pcc updated this revision to Diff 131168.Jan 23 2018, 4:14 PM
  • Use an inline implementation
pcc retitled this revision from IRGen: On MSVCRT platforms, implement __builtin_wmemcmp using memcmp. to IRGen: Emit an inline implementation of __builtin_wmemcmp on MSVCRT platforms..Jan 23 2018, 4:15 PM
pcc edited the summary of this revision. (Show Details)
majnemer added inline comments.
clang/lib/CodeGen/CGBuiltin.cpp
1771–1774 ↗(On Diff #131168)

Should we be more explicit about where 2 comes from? Maybe getTypeAlignInChars(WCharTy)?

pcc updated this revision to Diff 131171.Jan 23 2018, 4:59 PM
  • Ask the context for the type alignment
pcc marked an inline comment as done.Jan 23 2018, 4:59 PM
pcc added inline comments.
clang/lib/CodeGen/CGBuiltin.cpp
1771–1774 ↗(On Diff #131168)

Sure, works for me.

rnk accepted this revision.Jan 24 2018, 10:35 AM

lgtm

clang/lib/CodeGen/CGBuiltin.cpp
1771–1774 ↗(On Diff #131168)

I guess this would handle the (unreasonable) corner case of using wmemcmp, MSVC CRT, and -fno-short-wchar. O_o Anyway, looks good.

This revision is now accepted and ready to land.Jan 24 2018, 10:35 AM
This revision was automatically updated to reflect the committed changes.
pcc marked an inline comment as done.