[PATCH] D81676: [MSP430] Align the toolchain definition with the TI's msp430-gcc v9.2.0

[Kristina Bessonova via Phabricator via cfe-commits]

krisb added a comment.

Thank you!
LGTM, except some minor nits below.



================
Comment at: clang/lib/Driver/ToolChains/MSP430.cpp:154
 
+  SmallString<128> MultilibInclude(GCCInstallation.getInstallPath());
+  llvm::sys::path::append(MultilibInclude, "include");
----------------
I'd guard this by `if (GCCInstallation.isValid())` to avoid adding include directories with relative paths if `GCCInstallation.getInstallPath()` is empty.


================
Comment at: clang/lib/Driver/ToolChains/MSP430.cpp:239
+  Arg *SspFlag = Args.getLastArg(
+      options::OPT_fno_stack_protector, options::OPT_fstack_protector,
+      options::OPT_fstack_protector_all, options::OPT_fstack_protector_strong);
----------------
Is the check for `fno-stack-protector` necessary here? Looks as the checks for 'positive' options should be enough to do the trick.


================
Comment at: clang/test/Driver/msp430-toolchain.c:5
+
+// Test for include paths (cannot use -###)
+
----------------
This way looks okay to me, but I believe you should be able to check cc1 command (though -###) for `-internal-isystem`, right?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81676/new/

https://reviews.llvm.org/D81676