<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Sep 1, 2016 at 9:38 AM Sjoerd Meijer <<a href="mailto:sjoerd.meijer@arm.com">sjoerd.meijer@arm.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">SjoerdMeijer added inline comments.<br>
<br>
================<br>
Comment at: llvm/trunk/lib/Target/ARM/ARMAsmPrinter.cpp:455-461<br>
@@ -455,1 +454,9 @@<br>
+  if (TT.isOSBinFormatELF()) {<br>
+    if (!M.empty()) {<br>
+      // FIXME: this is a hack, but it is not more broken than<br>
+      // resetTargetOptions already was. The purpose of reading the target<br>
+      // options here is to read function attributes denormal and trapping-math<br>
+      // that we want to map onto build attributes.<br>
+      TM.resetTargetOptions(*M.begin());<br>
+    }<br>
     emitAttributes();<br>
----------------<br>
SjoerdMeijer wrote:<br>
> echristo wrote:<br>
> > No, this isn't the right place or way to do this.<br>
> ><br>
> > 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.<br>
> Ok, I will fix this.<br>
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 </blockquote><div><br></div><div>Yes, I would like you to fix this now :)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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).<br></blockquote><div><br></div><div>Sure. I'm not altogether happy the direction this patch is going in general and while you think it isn't more broken than it already was it has subtle wrong behavior in it as opposed to just being omitted.</div><div><br></div><div>Thanks.</div><div><br></div><div>-eric</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
<br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D24070" rel="noreferrer" target="_blank">https://reviews.llvm.org/D24070</a><br>
<br>
<br>
<br>
</blockquote></div></div>