This is an archive of the discontinued LLVM Phabricator instance.

[Sparc][Shave]: Empower the toolchain formerly known as SHAVE to do more.
ClosedPublic

Authored by dougk on Sep 1 2015, 2:57 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

dougk updated this revision to Diff 33739.Sep 1 2015, 2:57 PM
dougk retitled this revision from to [Sparc][Shave]: Empower the toolchain formerly known as SHAVE to do more..
dougk updated this object.
dougk added reviewers: jyknight, chandlerc.
dougk added a subscriber: cfe-commits.
dougk updated this revision to Diff 33778.Sep 1 2015, 8:40 PM
dougk updated this object.

Revised tests.

jyknight added inline comments.Sep 2 2015, 3:24 PM
lib/Driver/ToolChains.cpp
3936 ↗(On Diff #33778)

I wonder if it would be a good idea to add support for other suffixes like "le" to the findBiarchMultilibs or somewhere around there instead of hardcoding le/ here.

It looks like gcc has a whole bunch more things listed as possible MULTILIB_DIRNAMES for various targets. But LLVM seems to implement all the generality only for MIPS.

lib/Driver/ToolChains.h
934 ↗(On Diff #33778)

Why is this not "T.getArch() == llvm::Triple::shave"?

lib/Driver/Tools.cpp
9689 ↗(On Diff #33778)

Isn't whether this is needed dependent upon c++ vs not-c++, rather than rtems vs not-rtems?

dougk updated this revision to Diff 34682.Sep 14 2015, 9:00 AM
dougk updated this object.

Changes per review - but only a minimally correct change to finding the libraries. Using multilibs remains as a future task.
Also renamed SHAVE::Linker to Myriad::Linker.

dougk updated this revision to Diff 34729.Sep 14 2015, 1:58 PM

Delete the overrides of getTool() and buildAssembler() in MyriadToolChain.

jyknight added inline comments.Sep 14 2015, 3:39 PM
lib/Driver/ToolChains.cpp
3929 ↗(On Diff #34729)

Perhaps report an error if passed a cpu that's not sparc* or shave, since right now it'll get in here, and then just do something bogus?

lib/Driver/Tools.cpp
9754 ↗(On Diff #34729)

should probably be conditioned on being little endian.

9792 ↗(On Diff #34729)

as discussed, libg and libc are identical, so probably should use -lc here too for consistency.

dougk updated this revision to Diff 34998.Sep 17 2015, 7:58 AM
dougk marked 3 inline comments as done.

Use the same logic as Solaris::Solaris to die in the toolchain constructor if getArch() is unexpected.
Also don't hardcode -EL in linker.

jyknight accepted this revision.Sep 17 2015, 9:39 AM
jyknight edited edge metadata.

Other than minor comments, lgtm.

lib/Driver/ToolChains.cpp
3937 ↗(On Diff #34998)

I think you want something like:
D.Diag(diag::err_target_unsupported_arch) << Triple.getArchName() << "myriad"
since it is expected to be reachable via commandline args.

test/Driver/shave-toolchain.c
2 ↗(On Diff #34998)

Maybe some more check lines on this command, to verify that getCompilerSupportDir and getBuiltinLinkDir are returning useful values.

This revision is now accepted and ready to land.Sep 17 2015, 9:39 AM
This revision was automatically updated to reflect the committed changes.
dougk marked 2 inline comments as done.