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

[Eric Christopher via llvm-commits]

On Thu, Sep 1, 2016 at 9:38 AM Sjoerd Meijer <sjoerd.meijer at arm.com> wrote:

> 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


Yes, I would like you to fix this now :)


> 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).
>

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.

Thanks.

-eric


>
>
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D24070
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160901/bac6a2cb/attachment.html>