Target.cpp

Clang Frontend
1

Hi Daniel,

Sorry for the duplicate email, I keep forgetting to pay attention to the return address.

I’ve redone the patch without the Triple changes. The current triple mechanism seems a bit out-of-date, as the correct triple for 64-bit Windows appears to be “x86_64-pc-win32”, but we can leave that for whoever owns it. At some point, if Clang wants to be it’s own environment on Windows, we’ll probably have to revise this a bit to be able to do something other than default to Visual Studio for the “win32” OS-type, but this is probably good enough for now.

Thanks for dealing with this for me.

-John

P.S. I added an empty define for __declspec. Is this all that’s needed?

windows_targets.patch (6.36 KB)

2

Hi John,

Thanks for the refactoring, it looks good to me, I applied as r82621.

Hi Daniel,
Sorry for the duplicate email, I keep forgetting to pay attention to the
return address.
I've redone the patch without the Triple changes. The current triple
mechanism seems a bit out-of-date, as the correct triple for 64-bit Windows
appears to be "x86_64-pc-win32", but we can leave that for whoever owns it.

Ok.

At some point, if Clang wants to be it's own environment on Windows, we'll
probably have to revise this a bit to be able to do something other than
default to Visual Studio for the "win32" OS-type, but this is probably good
enough for now.

Yeah, we will probably want to go a long time before we care about
this. Adoption means being compatible with the existing environment,
as painful as it may be.

Thanks for dealing with this for me.

-John

P.S. I added an empty define for __declspec. Is this all that's needed?

Dunno...

By the way, I still don't have all my local patches in for testing on
Windows (there are basically 3, one to add count + not to
test/Scripts, one to add my local search headers, and one to hack
around stdint.h), but the current number of test failures is ~60,
which isn't too bad. A lot of the remaining ones are STL iterator
pickyness which should be easy to eliminate if someone sits down to
work through them.

Also, what happened with your patch to add MSVC search paths in a more
principled fashion? In my fuzzy memory I thought it had gone in, but I
didn't actually see it in the source.

- Daniel

3

Hello, John

Thanks for dealing with this for me.

Sorry, I haven't looked into this patch before - what's about long and
long long types for win64 targets?

4

Hi Daniel,

Thanks.

By the way, I still don’t have all my local patches in for testing on
Windows (there are basically 3, one to add count + not to
test/Scripts, one to add my local search headers, and one to hack
around stdint.h), but the current number of test failures is ~60,
which isn’t too bad. A lot of the remaining ones are STL iterator
pickyness which should be easy to eliminate if someone sits down to
work through them.

I still have the local patches you gave me before, which I was using, somewhat successfully, though the three of use trying the tests saw some different results. But that was a couple months ago, with me being pulled onto other projects and vacation, so I’ll see about running them again, if I can figure out how I was running them before. I’ll start looking at those test failures, hoping it’s not too far over my head. I do look forward to having the full solution for running the tests.

Also, what happened with your patch to add MSVC search paths in a more
principled fashion? In my fuzzy memory I thought it had gone in, but I
didn’t actually see it in the source.

Regarding the include path patch, it was kind of a hack job, mainly to facilitate our development. I think you or someone raised some objection about one part where when both vc80 and c90 are present (both the VS80COMNTOOLS and VS90COMNTOOLS environment variables are set) and I used the one Clang was built with. I kind of left it at that.

How do you think I should handle this case? Just use whichever is the later VS release?

I’ve enclosed a refreshed patch, in case you want to see it again. It still has some hard-coded paths like before, which is also why it’s kind of hackish.

-John

windows_include_paths.patch (3.25 KB)

5

Anton,

I’m not sure what you mean. Do you mean something about the DescriptionString member? I basically had the Windows targets inherit the one from the existing X86_64TargetInfo class. Does it need resetting for Windows? If so, I might need some help with that, as I don’t fully understant the string format there. Are there any docs for it? I didn’t see any in TargetInfo.*.

-John

6

Hello, John

I'm not sure what you mean. Do you mean something about the
DescriptionString member?

No, target data is the same (modulo some different alignments)

I basically had the Windows targets inherit the
one from the existing X86_64TargetInfo class. Does it need resetting for
Windows?

Yes. Windows uses LLP64 data model, and all other sane world LP64 :slight_smile:

Basically:
1. Windows:
sizeof(int) == sizeof(long) == 4
2. Everything else
sizeof(int) != sizeof(long)

see, e.g. Chapter 1: Introduction to Win32/Win64 | Microsoft Learn for
more information

7

Anton,

Here’s a patch for correcting the long size and align for Windows64.

-John

windows64_long.patch (437 Bytes)

8

Hi Daniel,

Thanks.

By the way, I still don't have all my local patches in for testing on

Windows (there are basically 3, one to add count + not to
test/Scripts, one to add my local search headers, and one to hack
around stdint.h), but the current number of test failures is ~60,
which isn't too bad. A lot of the remaining ones are STL iterator
pickyness which should be easy to eliminate if someone sits down to
work through them.

I still have the local patches you gave me before, which I was using,
somewhat successfully, though the three of use trying the tests saw some
different results. But that was a couple months ago, with me being pulled
onto other projects and vacation, so I'll see about running them again, if I
can figure out how I was running them before.

The way I am running them now is using the "clang-test" project in the
project files. That should be the "official" way for MSVC (from a
CMake build), and ideally will "just work" one day.

In fact, this is now working in buildbot too!
  http://google1.osuosl.org:8011/waterfall
The buildbot of course has a higher test failure count because it
doesn't get my local patches. The current count is 73 after the fixes
for count/not which just went in.

I'll start looking at those test failures, hoping it's not too far over my head.

I suspect a number of the issues are trivial things regarding to the
use of the debug STL library, which is much pickier on Windows. For
example, all the static analyzer tests were failing due to code like
'&*t.end()'. Then there is a group of tests that use /dev/null, I will
probably fix this in 'lit'. Finally there are issues with standard
include files not being usable. Those are the three major classes of
failures I am aware of.

I do look forward to
having the full solution for running the tests.

Also, what happened with your patch to add MSVC search paths in a more

principled fashion? In my fuzzy memory I thought it had gone in, but I
didn't actually see it in the source.
Regarding the include path patch, it was kind of a hack job, mainly to
facilitate our development. I think you or someone raised some objection
about one part where when both vc80 and c90 are present (both the
VS80COMNTOOLS and VS90COMNTOOLS environment variables are set) and I used
the one Clang was built with. I kind of left it at that.

How do you think I should handle this case? Just use whichever is the later
VS release?

For now, anything is better than nothing. Using the version the tools
was built with would be a fine start I guess, and perhaps closer to
what the user would want.

I've enclosed a refreshed patch, in case you want to see it again. It still
has some hard-coded paths like before, which is also why it's kind of
hackish.

Unless someone objects, I think the MSVC parts are worth putting it.
I'm not sure about the Cygwin/MinGW changes, were these intentional?

- Daniel

9

Daniel,

Unless someone objects, I think the MSVC parts are worth putting it.
I’m not sure about the Cygwin/MinGW changes, were these intentional?

Oh, I see there are problems there, as I deleted the lines picking up the C++ headers. This is problematic, with the include paths having compiler version numbers.

I’m actually at gcc 4.4.0 in MinGW (which doesn’t work for me, if you saw the email I just posted). Should I install 4.3.0 and put back the deleted paths? Or should we move to 4.4.0 and change the paths accordingly? I’ll wait to hear before I do anything.

-John

10

$ gcc -v t.cc 2>&1 | sed -n '/^ /p' | sed -n 2p
  /usr/include/c++/4.2.1

I have a dream, one day we will just compute the actual path up front during the build.

11

Here’s a revised patch for the Windows include paths, putting back the deleted lines for MinGW and Cygwin with a little revision. Note that I can’t test the MinGW and Cygwin stuff, because I can’t get those SKUs to build.

Also, another issue I brought up before but only remembered recently was that the old Python test runner wasn’t passing through the environment, so the Visual Studio environment variables (and C_INCLUDE_PATH) can’t be seen. I think you need to explicitly pass them through, but I wasn’t sure how to do it. Is this still an issue with the new script?

-John

windows_include_paths_2.patch (3.51 KB)

12

I have a dream, one day we will just compute the actual path up front during the build.

Do you mean the build of Clang, or when Clang runs?

Yeah, for the latter, I guess it’s just a matter of sitting down and doing it. Are there any directory walking tools in the code somewhere already?

-John

13

During the build…

14

Daniel,

Unless someone objects, I think the MSVC parts are worth putting it.

I'm not sure about the Cygwin/MinGW changes, were these intentional?
Oh, I see there are problems there, as I deleted the lines picking up the
C++ headers. This is problematic, with the include paths having compiler
version numbers.

I'm actually at gcc 4.4.0 in MinGW (which doesn't work for me, if you saw
the email I just posted). Should I install 4.3.0 and put back the deleted
paths? Or should we move to 4.4.0 and change the paths accordingly? I'll
wait to hear before I do anything.

For now I think its ok to have both.

- Daniel

15

Here's a revised patch for the Windows include paths, putting back the
deleted lines for MinGW and Cygwin with a little revision. Note that I
can't test the MinGW and Cygwin stuff, because I can't get those SKUs to
build.

Also, another issue I brought up before but only remembered recently was
that the old Python test runner wasn't passing through the environment, so
the Visual Studio environment variables (and C_INCLUDE_PATH) can't be seen.
I think you need to explicitly pass them through, but I wasn't sure how to
do it. Is this still an issue with the new script?

This is easy to change, just let me know which environment variables
to propagate.

- Daniel