chrib added a reviewer: jroelofs.
chrib added a comment.
Hi Martin,
maybe just a NIT, use --sysroot= rather than just --sysroot for consistency
with other tests.
Otherwise looks good to me, thanks (adding Jonathan as I'm not sure I can
accept)
Repository:
rC Clang
chrib added a comment.
Re-commit patch at r341547 with modified test case to use an explicit root tree
for libraries
Repository:
rC Clang
https://reviews.llvm.org/D51354
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
This revision was automatically updated to reflect the committed changes.
Closed by commit rC341373: Fix the -print-multi-directory flag to print the
selected multilib. (authored by chrib, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51354?vs=162846=163835#toc
chrib added a comment.
https://bugs.llvm.org/show_bug.cgi?id=21360
Repository:
rC Clang
https://reviews.llvm.org/D51354
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
chrib created this revision.
Herald added subscribers: cfe-commits, srhines.
Fix -print-multi-directory to print the selected multilib
Repository:
rC Clang
https://reviews.llvm.org/D51354
Files:
include/clang/Driver/ToolChain.h
lib/Driver/Driver.cpp
lib/Driver/ToolChains/Linux.cpp
chrib added a comment.
In https://reviews.llvm.org/D45814#1081203, @compnerd wrote:
> Do you have commit access or do you need someone to commit this on your
> behalf?
can you commit it for me please ? thanks.
Repository:
rC Clang
https://reviews.llvm.org/D45814
chrib added a comment.
Hi Saleem,
Thanks for your review. I have amended the patch to avoid checking the name on
the common path.
About your second comment, I'm not sure we have to fix -print-file-name= for an
equivalent problem since in case of empty parameter, we do a concatenation with
chrib updated this revision to Diff 144322.
chrib added a comment.
Move the non-null name check out of GetProgramPath and add a test case.
Assume that we trust the callers to check for non null ProgramName.
Repository:
rC Clang
https://reviews.llvm.org/D45814
Files:
lib/Driver/Driver.cpp
chrib created this revision.
Herald added a subscriber: cfe-commits.
Fix an assertion when -print-prog-name=
Repository:
rC Clang
https://reviews.llvm.org/D45814
Files:
lib/Driver/Driver.cpp
Index: lib/Driver/Driver.cpp
===
chrib updated this revision to Diff 135610.
chrib added a comment.
damn it last diff was the llvm part (https://reviews.llvm.org/D31139). Here is
the clang part.
sorry for the noise, still not experienced with Phabricator.
https://reviews.llvm.org/D31140
Files:
chrib updated this revision to Diff 135603.
chrib added a comment.
Herald added a subscriber: mehdi_amini.
hello, realizing that this has been stuck for a while now. did I answer all
the concerns ? rebased patch,
gentle ping :-)
many thanks,
Christian
https://reviews.llvm.org/D31140
chrib updated this revision to Diff 117139.
chrib added a comment.
Hello,
I realized that testing CCCisCXX for IsUnwindTablesDefault was not really good
and useless here, because we cannot treat C as C++ mode. In fact only the file
type matters, as seen in the tests.
So I'd like to amend my
chrib updated this revision to Diff 116176.
chrib added a comment.
Herald added a subscriber: javed.absar.
Changes since last revision:
- Check IsUnwindTablesDefault in ARM Toolchains that support
ExceptionHandling::ARM (not Darwin, NetBSD)
- Check input type with driver mode for C++ mode.
-
chrib added a comment.
Hello,
I didn't find an easy way factorize the change in IsUnwindTableDefault to
support the multiple ARM toolchains. After a quick check in Triple::arm in
Driver/Toochains, many seem impacted (excepted NetBSD and Darwin that uses
DwarfCFI or SJLG). So here is an
chrib added a comment.
oops yes of course. I forgot some bits while switching from testing the arch to
testing the platform.
indeed --target=arm-none-linux-gnueabihf lost its unwind info in c++.
Surprisingly not caught by the llvm tests, will add some.
thanks
https://reviews.llvm.org/D31140
chrib updated this revision to Diff 115176.
chrib added a comment.
Update IsUnwindTablesDefault to test fno-exceptions. (see Darwin.cpp)
Add comments.
https://reviews.llvm.org/D31140
Files:
lib/Driver/ToolChains/BareMetal.cpp
lib/Driver/ToolChains/BareMetal.h
Index:
chrib added a comment.
OK, we can refine so that unwind-table is also not generated for C++
-fno-exceptions.
To summarize this gives or C++ :
|| Unwind (table) | Exceptions (can throw) |
| default| Y| Y |
| fno-unwind | N
chrib updated this revision to Diff 114589.
chrib added a comment.
Hello Eli,
You were right, using IsUnwindTablesDefault is the correct way to abstract the
need for UnwindTables. I missed the relationship between the driver
munwind-table and GodeGenOpts.UnwindTables use in Codegen.
Here is a
chrib added a comment.
forgot to give the motivating figure. this brings a code size reduction of 9.5
% (geomean of text sections sizes) on coremarkpro -Oz for cortex-m3 eabi
https://reviews.llvm.org/D31140
___
cfe-commits mailing list
chrib planned changes to this revision.
chrib added a comment.
Yes, I also tried to look into this direction, but I did not found a clean way
from the driver to emit the attribute based on codegen options. Also
unwind-tables must not be the default. (for C this is what I want to
eliminate). In
chrib updated this revision to Diff 113846.
chrib added a comment.
Herald added a subscriber: kristof.beyls.
Rebase and cleanup NeedsUnwindTable for be variants.
https://reviews.llvm.org/D31140
Files:
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
Index:
chrib updated this revision to Diff 101675.
chrib added a comment.
- do not omit the frame pointer for netbsd-eabi and darwin-eabi
https://reviews.llvm.org/D31972
Files:
lib/Driver/ToolChains/Clang.cpp
test/Driver/frame-pointer.c
Index: test/Driver/frame-pointer.c
chrib added a comment.
OK, I have created a RFE tracker (BZ #32501). I will forward to cfe-dev.
Regarding the need to avoid table-based unwinding, the way to handle those
request for an explicit frame pointer when not required by the ABI is to use
-fno-omit-frame-pointer flag, catching up with
chrib added inline comments.
Comment at: lib/Driver/ToolChains/Clang.cpp:569
+ if (Triple.getEnvironment() == llvm::Triple::EABI) {
+switch (Triple.getArch()) {
efriedma wrote:
> chrib wrote:
> > efriedma wrote:
> > > Specifically checking for
chrib updated this revision to Diff 101543.
chrib added a comment.
Herald added a subscriber: kristof.beyls.
- Merge branch 'master' of ssh://codex.cro.st.com/llvm-arm/clang
- Don't need a frame pointer for EABIHF also (AAPCS)
https://reviews.llvm.org/D31972
Files:
chrib added inline comments.
Comment at: lib/Driver/ToolChains/Clang.cpp:569
+ if (Triple.getEnvironment() == llvm::Triple::EABI) {
+switch (Triple.getArch()) {
efriedma wrote:
> Specifically checking for "llvm::Triple::EABI" is suspicious... what are you
chrib added inline comments.
Comment at: test/CodeGen/arm-fp-eabi.c:1
+// RUN: %clang %s -target arm-none-eabi -O2 -S -o nofp.s
+// RUN: not grep '.setfp' nofp.s
ahatanak wrote:
> Instead of generating a .s file and looking for ".setfp", you can pass -###
> and
chrib updated this revision to Diff 96109.
chrib added a comment.
- Check not mdisable-fp-elim for arm eabi when optimizing
https://reviews.llvm.org/D31972
Files:
lib/Driver/ToolChains/Clang.cpp
test/Driver/frame-pointer.c
Index: test/Driver/frame-pointer.c
chrib updated this revision to Diff 95958.
chrib added a comment.
- Fix thinko in test
https://reviews.llvm.org/D31972
Files:
lib/Driver/ToolChains/Clang.cpp
test/CodeGen/arm-fp-eabi.c
Index: test/CodeGen/arm-fp-eabi.c
===
chrib updated this revision to Diff 95945.
chrib added a comment.
1. Updating https://reviews.llvm.org/D31972: Do not force the frame pointer by
default for ARM EABI #
Add test case
https://reviews.llvm.org/D31972
Files:
lib/Driver/ToolChains/Clang.cpp
test/CodeGen/arm-fp-eabi.c
chrib created this revision.
Herald added a subscriber: aemerson.
Do not force the frame pointer by default for ARM EABI
(bugzilla #32501)
https://reviews.llvm.org/D31972
Files:
lib/Driver/ToolChains/Clang.cpp
Index: lib/Driver/ToolChains/Clang.cpp
chrib updated this revision to Diff 94329.
chrib added a comment.
- NeedsUnwindTable check thumb arch
https://reviews.llvm.org/D31140
Files:
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
Index: lib/CodeGen/CodeGenModule.h
chrib updated this revision to Diff 93631.
chrib added a dependency: D31139: [LLVMbugs] [Bug 18710] Only generate
.ARM.exidx and .ARM.extab when needed with EHABI.
chrib added a comment.
1. Updating https://reviews.llvm.org/D31140: [LLVMbugs] [Bug 18710] Only
generate .ARM.exidx and
chrib added a comment.
Yes we can enable cantunwind with the nothrow gcc attribute when exceptions are
enabled
Forcing it in exceptions are not enabled (e.g for attribute cleanup) would
require -funwind-tables at function level anyway
So the flag should work, but conceptually I think you are
chrib added a comment.
In https://reviews.llvm.org/D31140#706411, @jroelofs wrote:
> Can you clarify the logic here? It's my understanding that:
>
> `-fno-exceptions` does *not* imply `-fno-unwind-tables`
>
> however:
>
> `-fno-unwind-tables` *does* imply that exceptions cannot be used on
35 matches
Mail list logo
