atrosinenko marked 3 inline comments as done.
atrosinenko added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/MSP430.cpp:154
+ SmallString<128> MultilibInclude(GCCInstallation.getInstallPath());
+ llvm::sys::path::append(MultilibInclude, "include");
----------------
krisb wrote:
> I'd guard this by `if (GCCInstallation.isValid())` to avoid adding include
> directories with relative paths if `GCCInstallation.getInstallPath()` is
> empty.
Fixed.
================
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);
----------------
krisb wrote:
> Is the check for `fno-stack-protector` necessary here? Looks as the checks
> for 'positive' options should be enough to do the trick.
While the "negative" option is not mentioned at all by the specs:
```
$ msp430-elf-gcc -dumpspecs | grep stack-protector
%{fstack-protector|fstack-protector-all|fstack-protector-strong|fstack-protector-explicit:-lssp_nonshared
-lssp}
```
it seems these options are already basically preprocessed by gcc driver:
```
$ msp430-elf-gcc -### empty.o 2>&1 | grep -Eo " -l[a-z_0-9]*" | tr "\n" " "
-lgcc -lmul_none -lc -lgcc -lcrt -lnosys -lgcc -lgcc
$ msp430-elf-gcc -### empty.o -fstack-protector 2>&1 | grep -Eo " -l[a-z_0-9]*"
| tr "\n" " "
-lssp_nonshared -lssp -lgcc -lmul_none -lc -lgcc -lcrt -lnosys -lgcc
-lgcc
$ msp430-elf-gcc -### empty.o -fstack-protector -fno-stack-protector 2>&1 |
grep -Eo " -l[a-z_0-9]*" | tr "\n" " "
-lgcc -lmul_none -lc -lgcc -lcrt -lnosys -lgcc -lgcc
```
and they seem to have the usual semantics of "the last one takes effect".
================
Comment at: clang/test/Driver/msp430-toolchain.c:5
+
+// Test for include paths (cannot use -###)
+
----------------
krisb wrote:
> This way looks okay to me, but I believe you should be able to check cc1
> command (though -###) for `-internal-isystem`, right?
It works, thank you! Will wait for unit test results on Windows.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81676/new/
https://reviews.llvm.org/D81676
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
