Page MenuHomePhabricator

[x32] Add X32 support to driver
ClosedPublic

Authored by pavel.v.chupin on Jun 17 2014, 10:23 AM.

Details

Summary

This is minimal change for frontend required to have "hello world" compiled
and working on x32 target (x86_64-linux-gnux32).
Note that this patch for backend is also required:
http://reviews.llvm.org/D4181

Diff Detail

Repository
rL LLVM

Event Timeline

pavel.v.chupin retitled this revision from to [x32] Add X32 support to driver.
pavel.v.chupin updated this object.
pavel.v.chupin edited the test plan for this revision. (Show Details)
pavel.v.chupin added a subscriber: Unknown Object (MLST).
compnerd added inline comments.Jun 18 2014, 11:01 PM
lib/Basic/Targets.cpp
3262 ↗(On Diff #10506)

I think chaining the assignment here similar to above might be nicer.

3273 ↗(On Diff #10506)

The two data layouts are identical except for the pointer size. Can you factor that out so that its more obvious and less redundant please?

lib/Driver/ToolChains.cpp
2977 ↗(On Diff #10506)

Can you add a check that arch is x86_64? Right now, you could make up ppc64---gnux32 if Im not mistaken.

lib/Driver/Tools.cpp
6730 ↗(On Diff #10506)

Can you do this inside the x86_64 case? Or is there a reason to make this its own case?

6928 ↗(On Diff #10506)

Can you extend this to check the arch?

7040 ↗(On Diff #10506)

Here as well.

Thanks for review. See next patchset.

lib/Basic/Targets.cpp
3262 ↗(On Diff #10506)

Done

3273 ↗(On Diff #10506)

DescriptionString is C-style string so next patchset change seems the best what can be done here.

lib/Driver/ToolChains.cpp
2977 ↗(On Diff #10506)

Done

lib/Driver/Tools.cpp
6730 ↗(On Diff #10506)

Done

6928 ↗(On Diff #10506)

Done

7040 ↗(On Diff #10506)

Done.

Changes after last review

Fix couple minor formatting issues

atanasyan requested changes to this revision.Jul 2 2014, 1:08 PM
atanasyan edited edge metadata.

Could you add tests to check this new functionality?

This revision now requires changes to proceed.Jul 2 2014, 1:08 PM
chandlerc edited edge metadata.Jul 2 2014, 1:19 PM

More importantly, this should wait for the llvmdev discussion much like the
other x32 patchset.

pavel.v.chupin edited edge metadata.

Added tests.
Redesigned findBiarchMultilibs for 3 arch (32,x32,64) support to pass more
tests.

Hi, it seems llvmdev discussion started week ago is over with some good comments made.
Can you please review this patch so we can proceed with these 2 patches merge and continue work on the next ones?

Please let me know if you think anything else is required to proceed here.
Thanks.

atanasyan accepted this revision.Jul 10 2014, 4:23 AM
atanasyan edited edge metadata.

LGTM

lib/Driver/ToolChains.cpp
1963 ↗(On Diff #11104)

s/isX32/IsX32/

This revision is now accepted and ready to land.Jul 10 2014, 4:23 AM
zinovy.nis closed this revision.Jul 10 2014, 8:36 AM
zinovy.nis updated this revision to Diff 11277.

Closed by commit rL212725 (authored by @zinovy.nis).