Page MenuHomePhabricator

Please use GitHub pull requests for new patches. Avoid migrating existing patches. Phabricator shutdown timeline

[clang] [hexagon] Add resource include dir

Authored by bcain on Jul 23 2021, 9:00 PM.



Diff Detail

Event Timeline

bcain requested review of this revision.Jul 23 2021, 9:00 PM
bcain created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJul 23 2021, 9:00 PM
bcain added a comment.Jul 23 2021, 9:03 PM

This patch is so far just for discussion, still needs tests, etc. The motivation for this patch is to be able to include the intrinsic header files installed in the resource dir.

I've taken inspiration from ToolChains/Linux.cpp here.

MaskRay requested changes to this revision.Jul 24 2021, 2:07 PM

Thanks for the patch. This makes the behavior closer to Linux.cpp . Request changes to clear my queue before a test is added...


To match Linux.cpp, this code block should be unconditional.

This revision now requires changes to proceed.Jul 24 2021, 2:07 PM
bcain added inline comments.Jul 26 2021, 9:59 PM

I *think* this is an intentional difference from Linux.cpp in order to suit hexagon-unknown-elf. @sidneym do you think we need this conditional behavior?

sidneym added inline comments.Jul 27 2021, 11:15 AM

The isMusl check is to distinguish between our internal c-library and musl c-library.

bcain updated this revision to Diff 364665.Aug 5 2021, 5:41 PM
bcain retitled this revision from DRAFT - [clang] [hexagon] Add resource include dir to [clang] [hexagon] Add resource include dir.

Add a test case, address failing test.

Thanks for the test. linux-cross.cpp has some -SAME: {{^}} and "[[RESOURCE]]/include" patterns which you may use

bcain updated this revision to Diff 365352.Aug 9 2021, 9:27 PM

Test case updates per suggestion.

MaskRay accepted this revision.Aug 9 2021, 9:41 PM
MaskRay added inline comments.

If you don't specify -resource-dir=%S/Inputs/resource_dir, under certain CMake configurations, the -resource-dir may be unreliable.


Some / may need to be \.

The Windows premerge test may give you a failure.
(That's why I just add UNSUPPORTED: system-windows to not deal with the backslash madness.)

This revision is now accepted and ready to land.Aug 9 2021, 9:41 PM
bcain updated this revision to Diff 365459.Aug 10 2021, 6:37 AM

Suggestions from Fangrui: override the resource-dir with an explicit arg, disable for windows.