Opened 5 years ago

Closed 4 years ago

Last modified 4 years ago

#11022 closed enhancement (fixed)

include/msvc/wx/setup.h does not handle 64 bit configurations

Reported by: nielsm Owned by: frm
Priority: normal Milestone:
Component: build Version: stable-latest
Keywords: MSVC setup.h Cc:
Blocked By: Blocking:
Patch: no

Description

The convenience-setup.h file for picking up the relevant setup.h for the actual configuration in use does not seem to take 64 bit builds (AMD64 or IA-64) into account, and assumes x86 builds. This requires a lot of manual shuffling files around if you wish to maintain 32 and 64 bit builds of wx side by side.

I am building with nmake as following:
nmake -f makefile.vc SHARED=0 BUILD=release for 32 bit
nmake -f makefile.vc SHARED=0 TARGET_CPU=AMD64 BUILD=release for 64 bit
The latter produces a dircetory named lib/vc_amd64_lib/ as opposed to the common lib/vc_lib/.

What I suggest is that include/msvc/wx/setup.h detects whether it is being built for 64 bit and includes from vc{,_amd64,_ia64}_{lib,dll} as appropriate.

Attachments (2)

msvc_setup_amd64_fix.patch download (1.8 KB) - added by frm 4 years ago.
setup.h.diff download (936 bytes) - added by dconnet 4 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 5 years ago by vadz

  • Keywords MSVC added
  • Status changed from new to confirmed

I agree that this should be done and it seems like it ought to be simple to do, there should be just an extra macro wxPLATFORM_SUFFIX which would be defined either as "" or "_amd64" (for example) and it should be concatenated with the rest of the library name components.

The most time-consuming part is actually testing that it works...

comment:2 follow-up: Changed 5 years ago by nielsm

There is actually a small complication with this: It's possible to build an AMD64 build without passing the TARGET_CPU=AND64 argument to the makefile, as long as the build environment is configured for AMD64 building (such as by running vcvarsall.bat amd64 with recent versions of MSVC), and in that case the _amd64 infix isn't added to the directory name.
I suppose there isn't reason to support that case, just like this setup.h doesn't support builds where the CFG= argument to the makefile was used.

comment:3 in reply to: ↑ 2 Changed 4 years ago by frm

  • Owner set to frm
  • Status changed from confirmed to accepted

Replying to nielsm:

There is actually a small complication with this: It's possible to build an AMD64 build without passing the TARGET_CPU=AND64 argument to the makefile, as long as the build environment is configured for AMD64 building (such as by running vcvarsall.bat amd64 with recent versions of MSVC), and in that case the _amd64 infix isn't added to the directory name.
I suppose there isn't reason to support that case, just like this setup.h doesn't support builds where the CFG= argument to the makefile was used.

I agree. In any case IIRC using the AMD64 build console _without_ the TARGET_CPU=AMD64 option will still not produce valid executables (at least for wx examples) because wxWidgets makefiles won't pass the /MACHINE:AMD64 option to the linker.

Anyway I'll attach here the patch I've created so far. As you can see in the comments I've added to msvc\setup.h I couldn't find a way to distinguish between IA64 and AMD64 builds but I think the fix is better than nothing (and covers >90% of use cases).

NOTE: the wiki page http://wiki.wxwidgets.org/Supporting_x64_and_Win32_within_one_solution#wxWidgets_2.9_Update needs to be updated as soon as this ticket is closed.

Changed 4 years ago by frm

comment:4 follow-up: Changed 4 years ago by dconnet

I haven't looked at the patch, but MS defines _M_IX86, _M_X64 and _M_IA64

I used these to tweak my local copy of setup.h to include "_amd64" in my wxCONCAT setup. (If you're interested, I can attach a diff of what I've locally done.

comment:5 in reply to: ↑ 4 Changed 4 years ago by frm

Replying to dconnet:

I haven't looked at the patch, but MS defines _M_IX86, _M_X64 and _M_IA64

interesting! these can be used as a proper fix to the definition of wxMACHINE_TYPE_SUFFIX then.

I used these to tweak my local copy of setup.h to include "_amd64" in my wxCONCAT setup. (If you're interested, I can attach a diff of what I've locally done.

yes, please. Attach your diff here so that we can evaluate a possibly better implementation of this additional feature to msvc\setup.h.

Changed 4 years ago by dconnet

comment:6 Changed 4 years ago by dconnet

I've attached setup.h.diff (generated using 'svn diff' on the trunk. I do not set things up for ia64, but you can easily see how I do it.

I use this, combined with my compilation script where I set 'COMPILER_PREFIX=vc...' and TARGET_CPU generates my lib directories in a nice consistent way.

comment:7 Changed 4 years ago by vadz

Thanks, I'm going to commit this then as there doesn't seem to be any reason not to, surely nobody is using wxWidgets only for x64 and so we don't risk breaking compatibility by doing this.

P.S. I'll also update the wiki.

comment:8 Changed 4 years ago by VZ

  • Resolution set to fixed
  • Status changed from accepted to closed

(In [63616]) Use correct directory names in msvc/wx/setup.h for 64 bit builds.

Use _amd64 and _ia64 suffixes if _M_X64 or _M_IA64 is defined.

Closes #11022.

comment:9 Changed 4 years ago by VZ

(In [63617]) Allow using version-specific vc prefix in msvc/wx/setup.h.

We still use just "vc" by default for compatibility but allow the user to
predefine either wxMSVC_VERSION or wxMSVC_VERSION_AUTO to use the specified or
version-dependent prefix instead. This is very convenient when using multiple
MSVC versions.

See #11022.

Note: See TracTickets for help on using tickets.