[PATCH] D24070: Setting fp trapping mode and denormal type

[Sjoerd Meijer via llvm-commits]

SjoerdMeijer added inline comments.

================
Comment at: llvm/trunk/include/llvm/CodeGen/CommandFlags.h:158-159
@@ +157,4 @@
+
+cl::opt<llvm::FPDenormal::DenormalType>
+DenormalType("denormal-fp-math",
+          cl::desc("Select which denormal numbers the code is permitted to require"),
----------------
arsenm wrote:
> Should this be per-FP type? We currently use a subtarget feature for this but need to split between f16/f32/f64
I now see how you are using it. We adopted the same approach as some other fp options (no NaNs, no infs, etc), which are also not per FP type. So to be honest, I don't know if they should. 

================
Comment at: llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp:455-461
@@ -455,1 +454,9 @@
+  if (TT.isOSBinFormatELF()) {
+    if (!M.empty()) {
+      // FIXME: this is a hack, but it is not more broken than
+      // resetTargetOptions already was. The purpose of reading the target
+      // options here is to read function attributes denormal and trapping-math
+      // that we want to map onto build attributes.
+      TM.resetTargetOptions(*M.begin());
+    }
     emitAttributes();
----------------
echristo wrote:
> No, this isn't the right place or way to do this.
> 
> You are going to want to look at iterating over the functions in a module in order to collect and collate options if you want to set a global asm marker.
Ok, I will fix this.


Repository:
  rL LLVM

https://reviews.llvm.org/D24070