[PATCH] D69878: Consoldiate internal denormal flushing controls

Mon Jan 6 07:01:30 PST 2020

arsenm marked an inline comment as done.
arsenm added a comment.

In D69878#1801508 <https://reviews.llvm.org/D69878#1801508>, @cameron.mcinally wrote:

> This is looking pretty good to me, but I'm ignoring some of the target specific code that I'm not familiar with.
>
> Is `denormal-fp-math` influenced by `-Ofast`? Or are there plans for that? Seems like `-Ofast` should imply DAZ and FTZ (if supported by target).

Yes, through the toolchain handling. I copied the logic for when crtfastmath is linked for the default mode for x86.

> I think we discussed this before, but it's worth repeating. If `denormal-fp-math` isn't specified, we default to IEEE behavior, right? When this lands in master, there could be an unexpected performance hit for targets that aren't paying attention. E.g. I want to use `denormal-fp-math` to toggle whether a FSUB(-0.0,X) is converted to a FNEG(X) in SelectionDAGBuilder.
> 
> Apologies in advance if this has been discussed recently. I've been distracted with another project for the passed few months...

Yes, ieee should be the default. The dependent patches start adding the attribute by default for platforms with flushing enabled with fast math

================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2311
+  bool TrappingMath = true;
                                     // overriden by ffp-exception-behavior?
   bool RoundingFPMath = false;
----------------
cameron.mcinally wrote:
> Last line of comment was not removed.
> 
> Also, is it safe to remove `TrappingMathPresent`? Is that part of the work-in-progress to support `ffp-exception-behavior`?
I think this is a rebase gone bad. The patch changing the strict math was revered and recommitted and I probably broke this

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69878/new/

https://reviews.llvm.org/D69878