[PATCH] D46135: [Driver, CodeGen] add options to enable/disable an FP cast optimization

Chandler Carruth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 26 14:53:25 PDT 2018


chandlerc added a comment.

In https://reviews.llvm.org/D46135#1080108, @spatel wrote:

> 3. Remove the 'no' version of the flag. Given the change in the default, this seems more natural to me, and it simplifies the patch/tests...but I might have been too pessimistic before and this is too optimistic? Let me know...


Please keep the `no-` version so that the flag can be toggled by appending to the command line.



================
Comment at: test/CodeGen/no-junk-ftrunc.c:1
+// RUN: %clang_cc1 -S -ffp-cast-overflow-workaround %s -emit-llvm -o - | FileCheck %s
+
----------------
spatel wrote:
> lebedev.ri wrote:
> > For a good measure, i'd add one more `RUN` line to test that it is currently the default.
> > (Yes, i noticed that it is already tested in `test/Driver/fast-math.c`)
> The driver alone is handling the default setting, so it passes this flag to the front-end only when we're going to disable the transform. Ie, the driver eats "-fno-fp-cast-overflow-workaround" and sends nothing in that case to the front-end. 
> 
> So there's not currently any case where the function attribute will be "fp-cast-overflow-workaround=false", but I left that as a possibility in case we decide to lift the limit at a finer granularity (scalar vs. vector etc).
> 
> I may be misunderstanding the question/suggestion - do we want the front-end to independently have a default setting?
How about a test that checks the attribute is *absent* in the default mode then?

I think it's useful to have the both sides of the test, however a particular side is spelled.


https://reviews.llvm.org/D46135





More information about the cfe-commits mailing list