[PATCH] D37891: Driver: hoist the `wchar_t` handling to the driver

[Matthias Braun via Phabricator via cfe-commits]

MatzeB added inline comments.


================
Comment at: lib/CodeGen/CodeGenModule.cpp:477
       Context.getTypeSizeInChars(Context.getWideCharType()).getQuantity();
-  assert((LangOpts.ShortWChar ||
-          llvm::TargetLibraryInfoImpl::getTargetWCharSize(Target.getTriple()) ==
----------------
compnerd wrote:
> compnerd wrote:
> > MatzeB wrote:
> > > rnk wrote:
> > > > @MatzeB ptal
> > > Can you find a new place for this assert()? Please do not just remove it!
> > > 
> > > For the backstory: Unfortunately I had to duplicate the wchar decision logic inside llvm (TargetLibraryInfoImpl::getTargetWCharSize() for cases where we just have the target triple available but need to know the size of wchar_t using library function. This means the logic in LLVM needs to be updated when support for new platforms is added but for people adding platform support it will not be obvious that they have the change LLVM/TargetLibraryInfo as well unless an assert() point them to there being a mismatch.
> > Sure, I'll try to see if I can find a suitable place or adjustment of it.  However, one thing to note is that the frontend does actually embed that into the IR metadata ("wchar_size"). 
> I think that if we try to retain this assertion, we need to update all the tests to ensure that they pass the arguments for selecting the `wchar_t` type.  The entire problem is that the backend view of this cannot correlate with what the user specified without passing it back to it.  The "wchar_size" metadata does exactly that.  Using that to perform the validation for the library function call.
This has been resolved with r314187 that made the assertion obsolete.


Repository:
  rL LLVM

https://reviews.llvm.org/D37891