[PATCH] D93395: [clang][cli] Remove -f[no-]trapping-math from -cc1 command line
Sjoerd Meijer via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Dec 21 03:02:11 PST 2020
SjoerdMeijer added inline comments.
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3080
}
Opts.setFPExceptionMode(FPEB);
----------------
jansvoboda11 wrote:
> SjoerdMeijer wrote:
> > jansvoboda11 wrote:
> > > The parsing of `OPT_ftrapping_math` and `OPT_fno_trapping_math` is immediately overwritten here.
> > Yeah, I did some work in this area some years ago, but that's a few years ago, and then in addition to that, we are talking about option handling in Clang which I always find very confusing... Just saying I can't remember too many details at this point.
> >
> > But starting with a first observation, you're saying that things are overwritten here, but they are different options. I.e., the deleted part honours `OPT_ftrapping_math`, and here exception modes are set based on `OPT_ffp_exception_behavior`. So, looks like there is some interaction here... Do we know how this should work?
> I see. The thing is, before this patch, we call `Opts.setFPExceptionMode` whenever we see either `OPT_ftrapping_math` or `OPT_fno_trapping_math`.
>
> But then, we **unconditionally** call `Opts.setFPExceptionMode(FPEB)` again here. That **always** overwrites what we previously did for `OPT_f[no_]trapping_math`, making that a dead code.
>
> `Opts.setFPExceptionMode()` doesn't do anything fancy, just assigns the argument to the `Opts.FPExceptionModel` member.
Ah yes, I actually missed that we always set `Opts.setFPExceptionMode(FPEB)` here! So that's clear now.
But looks like my previous question is still relevant how these options should work together? I now see that
`-ffp-exception-behavior` is a intel/microsoft special introduced in D62731.
And while currently we ignore `-fftrapping-math` here, but that just seems like a bug, it looks like we actually need to handle it here too?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93395/new/
https://reviews.llvm.org/D93395
More information about the cfe-commits
mailing list