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

[Sjoerd Meijer via llvm-commits]

SjoerdMeijer added inline comments.

================
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();
----------------
SjoerdMeijer wrote:
> 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.
I have looked at this again and got well confused (this is a messy bit of llvm). But I came to the same conclusion as earlier: it is not more broken than it already was. For example, options NoNaNsFPMath and NoInfsFPMath have the same problem. I think we all agree that the right approach is to "look at iterating over the functions in a module in order to collect and collate options". Do you want to fix this now? I started drafting a patch that fixes this issue for the options I introduced, but I am not happy with it. It does things in a different way than the other options, and I am not sure TargetMachine is the right place (I can upload the patch though if that is helpful).  




Repository:
  rL LLVM

https://reviews.llvm.org/D24070