echristo added a comment.
Replying to the inline comments. I know we're waiting on the linker stuff, but
Katya made me realize I hadn't replied to your comments.
-eric
================
Comment at: include/clang/Basic/DiagnosticDriverKinds.td:203-208
@@ +202,8 @@
+ InGroup<InvalidOrNonExistentDirectory>;
+def warn_drv_ps4_sdk_include : Warning<
+ "unable to find PS4 system headers directory, expected to be in '%0'">,
+ InGroup<InvalidOrNonExistentDirectory>;
+def warn_drv_ps4_sdk_lib : Warning<
+ "unable to find PS4 system libraries directory, expected to be in '%0'">,
+ InGroup<InvalidOrNonExistentDirectory>;
+
----------------
filcab wrote:
> echristo wrote:
> > Are the existing header warnings insufficient?
> Yes. We have an expected location and would like the whole "PS4 SDK headers"
> thing in there.
> But I can change it to be more general, like:
>
> def warn_drv_unable_find_directory_expected : Warning<
> "unable to find %0 directory, expected to be in '%1'">,
> InGroup<InvalidOrNonExistentDirectory>;
>
> That might be preferable. That way other toolchains can use it (and we can
> merge the "libraries" and "headers" warnings.
Seems totally reasonable :)
I can see it being something that you'd want to enable to make sure that
sysroot prefixes are paid attention to and, e.g. /usr/include is never touched.
================
Comment at: lib/Driver/Tools.cpp:3590-3591
@@ -3580,4 +3589,4 @@
Args.ClaimAllArgs(options::OPT_g_flags_Group);
if (Args.hasFlag(options::OPT_gcolumn_info, options::OPT_gno_column_info,
- /*Default*/ true))
+ /*Default*/ !Triple.isPS4CPU()))
CmdArgs.push_back("-dwarf-column-info");
----------------
filcab wrote:
> echristo wrote:
> > Hmm?
> We have different defaults from other platforms.
*nod* disabling column info yes? I have "opinions" on this, but it's not my
platform. Needs a comment.
================
Comment at: lib/Driver/Tools.cpp:3613
@@ -3603,3 +3612,3 @@
// backend.
- if (Args.hasArg(options::OPT_gdwarf_aranges)) {
+ if (Args.hasArg(options::OPT_gdwarf_aranges) || Triple.isPS4CPU()) {
CmdArgs.push_back("-backend-option");
----------------
filcab wrote:
> echristo wrote:
> > Ditto.
> Ditto, different defaults.
> But I guess I can hoist out the Triple.isPS4CPU() on both cases, if you
> prefer, like it's done for other toolchains like:
> bool IsWindowsMSVC = getToolChain().getTriple().isWindowsMSVCEnvironment();
Probably better to just handle it the same way as the column info I'd think.
Also, this is all terrible and needs to change, that said, not your problem. :)
================
Comment at: lib/Driver/Tools.h:783
@@ -782,1 +782,3 @@
+namespace PS4cpu {
+class LLVM_LIBRARY_VISIBILITY Assemble : public Tool {
----------------
filcab wrote:
> echristo wrote:
> > Hrm. PS4 maybe? PS4cpu seems to say that it's a custom cpu target and not a
> > custom environment?
> PS4cpu (or PS4CPU) is more consistent with how we've been naming things in
> open source.
Sure.
================
Comment at: lib/Frontend/InitHeaderSearch.cpp:325-344
@@ +324,22 @@
+ case llvm::Triple::PS4: {
+ // <isysroot> gets prepended later in AddPath().
+ std::string BaseSDKPath = "";
+ if (!HasSysroot) {
+ const char *envValue = getenv("SCE_PS4_SDK_DIR");
+ if (envValue)
+ BaseSDKPath = envValue;
+ else {
+ // HSOpts.ResourceDir variable contains the location of Clang's
+ // resource files.
+ // Assuming that Clang is configured for PS4 without
+ // --with-clang-resource-dir option, the location of Clang's resource
+ // files is <SDK_DIR>/host_tools/lib/clang
+ SmallString<128> P = StringRef(HSOpts.ResourceDir);
+ llvm::sys::path::append(P, "../../..");
+ BaseSDKPath = P.str();
+ }
+ }
+ AddPath(BaseSDKPath + "/target/include", System, false);
+ if (triple.isPS4CPU())
+ AddPath(BaseSDKPath + "/target/include_common", System, false);
+ }
----------------
kromanova wrote:
> echristo wrote:
> > This all seems odd, what's going on here?
> Our SDK layout structure is different from other platforms. Some of the PS4
> specific headers (e.g. graphic headers, etc) are placed into
> target/include_common directory and we want to have them in the search path.
You also check for ps4cpu in here?
http://reviews.llvm.org/D11279
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
