This is an archive of the discontinued LLVM Phabricator instance.

Compilation for Intel MCU (Part 2/3)
ClosedPublic

Authored by aturetsk on Apr 19 2016, 10:51 AM.

Details

Summary

This is the second patch required to support compilation for Intel MCU target (e.g. Intel(R) Quark(TM) micro controller D 2000).

When IAMCU triple is used:

  • Recognize and use IAMCU GCC toolchain
  • Set up include paths
  • Forbid C++

The linker-related changes will be in the last part.

Diff Detail

Event Timeline

aturetsk updated this revision to Diff 54221.Apr 19 2016, 10:51 AM
aturetsk retitled this revision from to Compilation for Intel MCU (Part 2/3).
aturetsk updated this object.
aturetsk added reviewers: rsmith, bruno, bkramer.
mgrang added a subscriber: mgrang.Apr 19 2016, 11:10 AM
mgrang added inline comments.
lib/Driver/Tools.cpp
583

Is it better to move this above in the else part of the condition "if (!IsIAMCU)"?

3624

nitpick: Missing space between if and (

aturetsk updated this revision to Diff 54341.Apr 20 2016, 4:27 AM

Fixed the remarks.

Hi Mandeep,
Thanks for the review.

lib/Driver/ToolChains.cpp
2153

Does anybody know if 'libgcc.a' could be used instead of 'crtbegin.o' for all targets?
If so we can avoid changing FilterNonExistent so much...

lib/Driver/Tools.cpp
583

Done.

3624

Fixed.

bruno edited edge metadata.May 11 2016, 9:50 AM

Hi,

include/clang/Basic/DiagnosticDriverKinds.td
154

Are you sure there's no equivalente for this already? I'm surprised!

lib/Driver/Tools.cpp
300

Tidy up this declaration with others, no need for two newlines here.

575

Use braces for the "else" and move the comment somewhere inside the braces

aturetsk updated this revision to Diff 57017.May 12 2016, 4:23 AM
aturetsk edited edge metadata.

Fix the remarks.

Hi Bruno,
Thanks for the review.

include/clang/Basic/DiagnosticDriverKinds.td
154

Yes. I've found nothing similar.

lib/Driver/Tools.cpp
302

Done.

575

Done.

Hi,

lib/Driver/Tools.cpp
3625

Taking a look at this again I don't think there's a real need for a new diagnostic here; instead of adding diag::err_drv_cxx_not_supported, you can do something similar to:

D.Diag(diag::err_drv_clang_unsupported) << "C++ is not supported with -miamcu"

Otherwise, LGTM!

aturetsk added inline comments.May 26 2016, 5:23 AM
lib/Driver/Tools.cpp
3625

The best thing I could come up with is this:

D.Diag(diag::err_drv_clang_unsupported) << "C++ for IAMCU";

This code makes the error message look like this:

error: the clang compiler does not support 'C++ for IAMCU'

As you can see the message is a bit crooked. Do you thinks it's better to keep it this way?

bruno added inline comments.May 26 2016, 11:29 AM
lib/Driver/Tools.cpp
3625

Sorry, but I didn't mean you should change the message. My only point here is that I don't see the need for adding a new diagnostic, you can still keep something more similar with your old intent, example:

D.Diag(diag::err_drv_clang_unsupported) << "C++ for target" << getToolChain().getTriple().str();
aturetsk added inline comments.May 26 2016, 11:38 AM
lib/Driver/Tools.cpp
3625

My concern is not the change in the message :) My concern is redundant quotation marks which look a bit ugly and out of place.
But if you're OK with them, so would I.

aturetsk added inline comments.Jun 15 2016, 7:03 AM
lib/Driver/Tools.cpp
3625

Bruno, are you ok if the message will be:

error: the clang compiler does not support 'C++ for IAMCU'

If so I'll proceed with committing the changes.

bruno accepted this revision.Jun 15 2016, 11:12 AM
bruno edited edge metadata.

Sure! LGTM

This revision is now accepted and ready to land.Jun 15 2016, 11:12 AM
This revision was automatically updated to reflect the committed changes.