This is an archive of the discontinued LLVM Phabricator instance.

CodeGenModule: Always output wchar_size; check LLVM assumptions.
ClosedPublic

Authored by MatzeB on May 8 2017, 3:39 PM.

Details

Summary

llvm::TargetLibraryInfo needs to know the size of wchar_t to work on
functions like wcslen. This patch changes clang to always emit the
wchar_size module flag (it would only do so for ARM previously).
This also adds an assert() to ensure the LLVM defaults based on the
target triple are in sync with clang.

Diff Detail

Repository
rL LLVM

Event Timeline

MatzeB created this revision.May 8 2017, 3:39 PM
efriedma edited edge metadata.May 8 2017, 3:54 PM

This makes sense to me, but I'll wait to see if anyone else has a comment.

test/CodeGen/wchar-size.c
2 ↗(On Diff #98225)

Maybe add a comment to the test noting that -fno-short-wchar has no effect on Windows.

rnk edited edge metadata.May 8 2017, 4:12 PM

This seems like the kind of thing we would normally express in DataLayout.

MatzeB added a comment.May 8 2017, 5:14 PM
In D32982#749215, @rnk wrote:

This seems like the kind of thing we would normally express in DataLayout.

As far as I can tell DataLayout is designed to talk about llvm types (llvm::Type), talking about types/typedefs at the source language level seems out of scope.

ping. Do we really need wchar_t size in DataLayout to move this forward? I'm not convinced that is the right place for it...

MatzeB updated this revision to Diff 99344.May 17 2017, 1:48 PM

Add comment to test

MatzeB marked an inline comment as done.May 17 2017, 1:48 PM
rnk accepted this revision.May 17 2017, 3:07 PM

I guess this is better than DataLayout. Also, the user experience of mixing two TUs with different -fshort-wchar settings during LTO is better.

lib/CodeGen/CodeGenModule.cpp
473 ↗(On Diff #99344)

Is this what clang-format does?

This revision is now accepted and ready to land.May 17 2017, 3:07 PM
This revision was automatically updated to reflect the committed changes.