Page MenuHomePhabricator

Do not define the gcc style target specific pre-defined preprocessor macros in clang-cl
AbandonedPublic

Authored by ehsan on Jul 7 2014, 9:57 PM.

Details

Reviewers
hansw
rnk

Diff Detail

Event Timeline

ehsan updated this revision to Diff 11139.Jul 7 2014, 9:57 PM
ehsan retitled this revision from to Do not define the gcc style target specific pre-defined preprocessor macros in clang-cl.
ehsan updated this object.
ehsan edited the test plan for this revision. (Show Details)
ehsan added reviewers: rnk, hansw.
ehsan added a subscriber: Unknown Object (MLST).

This patch looks OK, however it doesn't allow users of clang to actually use the builtins; the backend will still error out.

That's OK, the main intention behind this patch is to unbreak code that assumes gcc specific things based on #ifdef i386 and friends...

With that in mind, does this look good to land?

rnk edited edge metadata.Jul 9 2014, 3:58 PM

How much code breaks without this? Do we really want to do this? My inclination is to get by without it. In particular, libyuv has made changes to not assume that x86_64 implies GNUC functionality.

lib/Basic/Targets.cpp
2592

This condition is incorrect, Microsoft builds apps such as Office for Mac with clang with -fms-extensions. They very likely include system headers that rely on x86_64 &co. This should probably be:

if (MicrosoftExt)
  ... _M_* macros
if (MSVCCompat)
  return;
...

In our code base I think it's only occurrence of this problem, but I do
expect this to be the sort of thing that breaks more than just libyuv in
the wild. That being said, I'll leave it up to you whether we should take
this or not, and will address the comment below when landing if we decide
to to do so.

Cheers,

Ehsan
http://ehsanakhgari.org/

ehsan abandoned this revision.Jul 22 2014, 7:59 AM