martell added a comment.
I also want to note a small addition armb and thumbeb for NetBSD.
They were missed in the previous version.
Just waiting for tests to pass now.
Repository:
rL LLVM
https://reviews.llvm.org/D39673
___
cfe-commits mailing
martell added a comment.
@mstorsjo yup, I added a comment on the commit about the failure here
https://reviews.llvm.org/rL319294
I have already fixed both issues.
Will re apply shortly.
Repository:
rL LLVM
https://reviews.llvm.org/D39673
___
mstorsjo added inline comments.
Comment at: cfe/trunk/lib/Driver/ToolChains/FreeBSD.cpp:368
+getTriple().getArch() == llvm::Triple::thumb)
+ return llvm::ExceptionHandling::SjLj;
case llvm::Triple::GNUEABIHF:
This seems to need a fallthrough
mstorsjo added inline comments.
Comment at: cfe/trunk/lib/Driver/ToolChain.cpp:457
+ if (Triple.isOSWindows())
+return llvm::ExceptionHandling::WinEH;
+ return llvm::ExceptionHandling::None;
It looks like this broke some buildbot after all - see e.g.
This revision was automatically updated to reflect the committed changes.
Closed by commit rL319294: Toolchain: Normalize dwarf, sjlj and seh eh
(authored by martell).
Changed prior to commit:
https://reviews.llvm.org/D39673?vs=124695=124696#toc
Repository:
rL LLVM
martell added a comment.
landing this as it was previous approved by reid in this state and again re
checked by martin.
Repository:
rL LLVM
https://reviews.llvm.org/D39673
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
martell updated this revision to Diff 124695.
martell added a comment.
- disregard my last comment, lets go with seh on all windows targets.
Repository:
rL LLVM
https://reviews.llvm.org/D39673
Files:
docs/ClangCommandLineReference.rst
include/clang/Basic/LangOptions.def
martell added inline comments.
Comment at: lib/Driver/ToolChain.cpp:458
+if (Triple.getArch() == llvm::Triple::x86)
+ return llvm::ExceptionHandling::DwarfCFI;
+else
mstorsjo wrote:
> I'd suggest braces around the outer if statement.
>
> But is
mstorsjo added inline comments.
Comment at: lib/Driver/ToolChain.cpp:458
+if (Triple.getArch() == llvm::Triple::x86)
+ return llvm::ExceptionHandling::DwarfCFI;
+else
I'd suggest braces around the outer if statement.
But is there any point to this
martell updated this revision to Diff 124692.
martell added a comment.
address reid's comment
update to HEAD
Repository:
rL LLVM
https://reviews.llvm.org/D39673
Files:
docs/ClangCommandLineReference.rst
include/clang/Basic/LangOptions.def
include/clang/Driver/Options.td
rnk requested changes to this revision.
rnk added inline comments.
This revision now requires changes to proceed.
Comment at: lib/Driver/ToolChain.cpp:450-458
+ const llvm::Target *TT = llvm::TargetRegistry::lookupTarget(TS, Error);
+ if (!TT)
+return
mstorsjo accepted this revision.
mstorsjo added a comment.
Looks really good to me now! @rnk?
Repository:
rL LLVM
https://reviews.llvm.org/D39673
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
martell added a comment.
In https://reviews.llvm.org/D39673#931861, @rnk wrote:
> We have to know the EH model before pre-processing, and that shouldn't rely
> on LLVM TargetOptions. We can probably reuse the `llvm::ExceptionHandling`
> enum instead of these various overlapping booleans, if
martell updated this revision to Diff 123883.
martell edited the summary of this revision.
martell added a comment.
Herald added subscribers: kristof.beyls, emaste, aemerson.
fold USESjLjExceptions into GetExceptionModel.
do a best effort to check the llvm default exception model for the triple
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
Yep, looks good
Repository:
rL LLVM
https://reviews.llvm.org/D39673
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
mstorsjo added a comment.
No objection from me about committing this now, although I have some minor
comments (that possibly can be taken care of without reposting and
re-reviewing). Still ok with @rnk?
Comment at: lib/Frontend/InitPreprocessor.cpp:686
+ else if
rnk added a comment.
In https://reviews.llvm.org/D39673#931487, @martell wrote:
> Just as a note there is still a lot to be desired here. I do not particularly
> like the `UseSEHExceptions` function default and the actual macro definition
> guards should be based on the current
martell added a comment.
Just as a note there is still a lot to be desired here. I do not particularly
like the `UseSEHExceptions` function default and the actual macro definition
guards should be based on the current `ExceptionModel` because we set that in
`lib/CodeGen/BackendUtil.cpp`. This
martell added a comment.
In https://reviews.llvm.org/D39673#929986, @mstorsjo wrote:
> I'm not sure if this is the right thing to do. Since the exception handling
> model more or less also defines what ABI the code conforms to, I can see it
> being useful to know what exception handling mode
martell updated this revision to Diff 123758.
martell added a comment.
updated to HEAD.
added a NOT msvc test
Repository:
rL LLVM
https://reviews.llvm.org/D39673
Files:
docs/ClangCommandLineReference.rst
include/clang/Basic/LangOptions.def
include/clang/Driver/Options.td
mstorsjo added inline comments.
Comment at: lib/Frontend/InitPreprocessor.cpp:684
+else if (TI.getTriple().isThumb() || TI.getTriple().isARM())
+ Builder.defineMacro("__ARM_DWARF_EH__");
+ }
martell wrote:
> mstorsjo wrote:
> > Won't this start setting
mstorsjo added a comment.
In https://reviews.llvm.org/D39673#929536, @martell wrote:
> When doing that I noticed there is something really strange about the
> existing macro defines. I assume they should only be defined when exceptions
> is enabled.
> This is by default in c++ mode of with
martell added inline comments.
Comment at: lib/Frontend/InitPreprocessor.cpp:684
+else if (TI.getTriple().isThumb() || TI.getTriple().isARM())
+ Builder.defineMacro("__ARM_DWARF_EH__");
+ }
mstorsjo wrote:
> Won't this start setting this define also on
mstorsjo added inline comments.
Comment at: lib/Frontend/InitPreprocessor.cpp:684
+else if (TI.getTriple().isThumb() || TI.getTriple().isARM())
+ Builder.defineMacro("__ARM_DWARF_EH__");
+ }
Won't this start setting this define also on platforms where
martell requested review of this revision.
martell added a comment.
Thanks for the review @rnk . I addressed the comment nit you had.
@mstorsjo I updated this to also handle thumb on arm.
When doing that I noticed there is something really strange about the existing
macro defines. I assume
martell updated this revision to Diff 123486.
martell added a comment.
Herald added subscribers: JDevlieghere, javed.absar.
updated to handle dwarf exceptions on arm.
Repository:
rL LLVM
https://reviews.llvm.org/D39673
Files:
docs/ClangCommandLineReference.rst
mstorsjo added a comment.
https://reviews.llvm.org/D39533 was committed now, so before committing, make
sure that the `__ARM_DWARF_EH__` (that was added in that commit) only gets set
while dwarf is enabled (which now is the default for mingw/arm but not
msvc/arm).
Repository:
rL LLVM
mstorsjo added inline comments.
Comment at: lib/Frontend/InitPreprocessor.cpp:683
+ if (LangOpts.SEHExceptions)
+Builder.defineMacro("__SEH__");
Please define `__ARM_DWARF_EH__` if dwarf is enabled on arm, see D39533, as I
commented earlier. (Sorry, I
rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
Looks good, sorry for the delay!
Comment at: include/clang/Basic/LangOptions.def:129
LANGOPT(SjLjExceptions, 1, 0, "setjmp-longjump exception handling")
martell added a comment.
Ping :)
Repository:
rL LLVM
https://reviews.llvm.org/D39673
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
martell added a comment.
@rnk ping :)
Repository:
rL LLVM
https://reviews.llvm.org/D39673
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
mstorsjo added a comment.
In https://reviews.llvm.org/D39673#917524, @martell wrote:
> I mostly expect this to be useful where environment has builds all 3 versions
> of the libunwind. where the library is named differently based on the eh
> model.
> e.g. `libunwind-sjlj.a` `libunwind-seh.a`
martell added a comment.
In https://reviews.llvm.org/D39673, @martell wrote:
> Long term I would like the MinGW Driver to load a different libunwind
> depending on the exception model but that discussion might still be a little
> off.
In https://reviews.llvm.org/D39673#917510, @mstorsjo
mstorsjo added a comment.
In https://reviews.llvm.org/D39673#917506, @martell wrote:
> @mstorsjo I think this should help with issues like having to set the default
> for ARM to DWARF.
Btw, in addition to making setting `__SEH__` conditional, it should also
instead set `__ARM_DWARF_EH__` on
mstorsjo added a comment.
In https://reviews.llvm.org/D39673#917506, @martell wrote:
> @mstorsjo I think this should help with issues like having to set the default
> for ARM to DWARF.
> If you can set it at run time then it should be a lot easier to just set
> that in the clang wrapper
martell added a comment.
@mstorsjo I think this should help with issues like having to set the default
for ARM to DWARF.
If you can set it at run time then it should be a lot easier to just set that
in the clang wrapper scripts.
Long term I would like the MinGW Driver to load a different
martell updated this revision to Diff 121832.
martell added a comment.
Add command line documentation references
Repository:
rL LLVM
https://reviews.llvm.org/D39673
Files:
docs/ClangCommandLineReference.rst
include/clang/Basic/LangOptions.def
include/clang/Driver/Options.td
martell created this revision.
Herald added a subscriber: aprantl.
Adds `-fseh-exceptions` and `-fdwarf-exceptions` to compliment
`-fsjlj-exceptions`.
This makes the exception personality configurable at runtime rather then just
compile time.
If nothing is passed to cc1 we default to
38 matches
Mail list logo
