llvm dev - Oct 2021 - [RFC] Eliminating non-IR floating-point controls in the selection DAG

Hi everyone,

This is related to the recent thread about fp-contract and front end pragma
controls, but I want to generalize the discussion in terms of how the
target-independent codegen in the backend is implemented.

Until sometime in 2017 (I think) the fast-math flags were not passed through to
the Selection DAG, and so the only ways to control floating-point behavior were
through settings in the TargetOptions or by settings function attributes. Since
2017, however, the fast-math flags have been attached to floating-point nodes in
the selection DAG. This leads to some ambiguous situations where the
TargetOptions or function attributes can override the absence of fast-math flags
on individual nodes. An example of this is the fp-contract setting. If a source
file is compiled with clang using the '-ffp-contract=fast' setting but
the file contains either "#pragma STDC FP_CONTRACT OFF" or
"#pragma clang fp contract(off)" the front end will generate IR
without the 'contract' fast-math flag set, but the X86 backend will
generate an FMA instruction anyway.

https://godbolt.org/z/dov6EcE8G

This is particularly bad in the case of CUDA, because CUDA uses fp-contract=fast
by default. So, the user's code can explicitly say "don't generate
fma here" and the compiler will respond, "meh, I think I will
anyway."

https://godbolt.org/z/c4h1nK9M3

There are other cases where the backend code will check for
TargetOption::UnsafeFPMath for things like reassociation that can be represented
using fast-math flags.

That brings me to the RFC part of my message. I'd like to start updating the
backend so that it doesn't do things like this. As a general principle, I
would say, "All semantics must be represented in the IR and the backend
must respect the IR semantics." And a corollary: "Anything which can
be represented at the instruction level must be represented at the instruction
level." This corollary would eliminate potential conflicts between function
attributes (like "unsafe-fp-math") and individual IR instructions.

As a first step toward this goal, I've prepared a patch which closes the
back door for fp-contract control.

https://reviews.llvm.org/D112760

This patch is currently incomplete, in as much as I didn't update failing
tests for several target architectures. I did update the X86 and AMDGPU tests to
provide examples of how they can be made to work. I will fix the rest if we
decide this is the correct direction. There is a failing CUDA test in the clang
front end that I think will require a different approach involving some driver
changes to get clang to generate IR for the semantics it intends rather than
setting an option and counting on the backend to disregard the IR.

Thanks in advance for any feedback!

-Andy

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20211029/b89260a1/attachment.html>

Generally, this makes a lot of sense to me.  If we have semantics 
expressible in IR, we should use that as the source of truth.  Not 
really my area, so I don't have any opinion on the specific 
implementation plan.

Philip

On 10/28/21 5:22 PM, Kaylor, Andrew via llvm-dev wrote:
>
> Hi everyone,
>
> This is related to the recent thread about fp-contract and front end 
> pragma controls, but I want to generalize the discussion in terms of 
> how the target-independent codegen in the backend is implemented.
>
> Until sometime in 2017 (I think) the fast-math flags were not passed 
> through to the Selection DAG, and so the only ways to control 
> floating-point behavior were through settings in the TargetOptions or 
> by settings function attributes. Since 2017, however, the fast-math 
> flags have been attached to floating-point nodes in the selection DAG. 
> This leads to some ambiguous situations where the TargetOptions or 
> function attributes can override the absence of fast-math flags on 
> individual nodes. An example of this is the fp-contract setting. If a 
> source file is compiled with clang using the ‘-ffp-contract=fast’ 
> setting but the file contains either “#pragma STDC FP_CONTRACT OFF” or 
> “#pragma clang fp contract(off)” the front end will generate IR 
> without the ‘contract’ fast-math flag set, but the X86 backend will 
> generate an FMA instruction anyway.
>
> https://godbolt.org/z/dov6EcE8G <https://godbolt.org/z/dov6EcE8G>
>
> This is particularly bad in the case of CUDA, because CUDA uses 
> fp-contract=fast by default. So, the user’s code can explicitly say 
> “don’t generate fma here” and the compiler will respond, “meh, I think 
> I will anyway.”
>
> https://godbolt.org/z/c4h1nK9M3 <https://godbolt.org/z/c4h1nK9M3>
>
> There are other cases where the backend code will check for 
> TargetOption::UnsafeFPMath for things like reassociation that can be 
> represented using fast-math flags.
>
> That brings me to the RFC part of my message. I’d like to start 
> updating the backend so that it doesn’t do things like this. As a 
> general principle, I would say, “All semantics must be represented in 
> the IR and the backend must respect the IR semantics.” And a 
> corollary: “Anything which can be represented at the instruction level 
> must be represented at the instruction level.” This corollary would 
> eliminate potential conflicts between function attributes (like 
> "unsafe-fp-math") and individual IR instructions.
>
> As a first step toward this goal, I’ve prepared a patch which closes 
> the back door for fp-contract control.
>
> https://reviews.llvm.org/D112760 <https://reviews.llvm.org/D112760>
>
> This patch is currently incomplete, in as much as I didn’t update 
> failing tests for several target architectures. I did update the X86 
> and AMDGPU tests to provide examples of how they can be made to work. 
> I will fix the rest if we decide this is the correct direction. There 
> is a failing CUDA test in the clang front end that I think will 
> require a different approach involving some driver changes to get 
> clang to generate IR for the semantics it intends rather than setting 
> an option and counting on the backend to disregard the IR.
>
> Thanks in advance for any feedback!
>
> -Andy
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20211028/35a8b4db/attachment.html>

Hi Andrew,

IIRC, the fp-contract change was to fix some broken code but not break
more. Copying Sebastian who was working on that at that time.

I agree we shouldn't have overriding behaviour flags in the back-end if the
IR explicitly says what it wants. But I'd be cautious as to move all such
flags to instructions.

First, this would be a behavioural change that needs the IR to change, so
would affect every LLVM IR front-end, which makes it a pervasive change
throughout the downstream users. So, if we decide we want to do this, we
need to replace the current mess with a consistent implementation that wont
break everybody else's.

Second, module/function flags can control fine-grain behaviour without
bloating the IR. I don't know how the instruction flag changes the binary
representation, it's probably very small anyway, but so are the
module/function ones, so overall, a definite increase in size.

Finally, I think we need to separate the IR from DAG/MIR behaviour. It
seems to me that the target option is what overrides the behaviour, not
function/module options, so we should worry about the targets' behaviour,
not at which level the flag is set.

There's a perfectly valid solution that has module/function/instruction
flags controlling behaviour, with the most specific overriding the least
specific, and none of that overridden by the target. This means we can
still use the same IR flags in the same way (thus not forcing all
front-ends to change) and still correct the behaviour by not making the
target ignore all that.

Does any of that make any sense?

cheers,
--renato



On Fri, 29 Oct 2021 at 01:22, Kaylor, Andrew via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Hi everyone,
>
>
>
> This is related to the recent thread about fp-contract and front end
> pragma controls, but I want to generalize the discussion in terms of how
> the target-independent codegen in the backend is implemented.
>
>
>
> Until sometime in 2017 (I think) the fast-math flags were not passed
> through to the Selection DAG, and so the only ways to control
> floating-point behavior were through settings in the TargetOptions or by
> settings function attributes. Since 2017, however, the fast-math flags have
> been attached to floating-point nodes in the selection DAG. This leads to
> some ambiguous situations where the TargetOptions or function attributes
> can override the absence of fast-math flags on individual nodes. An example
> of this is the fp-contract setting. If a source file is compiled with clang
> using the ‘-ffp-contract=fast’ setting but the file contains either
> “#pragma STDC FP_CONTRACT OFF” or “#pragma clang fp contract(off)” the
> front end will generate IR without the ‘contract’ fast-math flag set, but
> the X86 backend will generate an FMA instruction anyway.
>
>
>
> https://godbolt.org/z/dov6EcE8G
>
>
>
> This is particularly bad in the case of CUDA, because CUDA uses
> fp-contract=fast by default. So, the user’s code can explicitly say “don’t
> generate fma here” and the compiler will respond, “meh, I think I will
> anyway.”
>
>
>
> https://godbolt.org/z/c4h1nK9M3
>
>
>
> There are other cases where the backend code will check for
> TargetOption::UnsafeFPMath for things like reassociation that can be
> represented using fast-math flags.
>
>
>
> That brings me to the RFC part of my message. I’d like to start updating
> the backend so that it doesn’t do things like this. As a general principle,
> I would say, “All semantics must be represented in the IR and the backend
> must respect the IR semantics.” And a corollary: “Anything which can be
> represented at the instruction level must be represented at the instruction
> level.” This corollary would eliminate potential conflicts between function
> attributes (like "unsafe-fp-math") and individual IR
instructions.
>
>
>
> As a first step toward this goal, I’ve prepared a patch which closes the
> back door for fp-contract control.
>
>
>
> https://reviews.llvm.org/D112760
>
>
>
> This patch is currently incomplete, in as much as I didn’t update failing
> tests for several target architectures. I did update the X86 and AMDGPU
> tests to provide examples of how they can be made to work. I will fix the
> rest if we decide this is the correct direction. There is a failing CUDA
> test in the clang front end that I think will require a different approach
> involving some driver changes to get clang to generate IR for the semantics
> it intends rather than setting an option and counting on the backend to
> disregard the IR.
>
>
>
> Thanks in advance for any feedback!
>
>
>
> -Andy
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20211029/35f3aade/attachment.html>

On 28 Oct 2021, at 20:22, Kaylor, Andrew wrote:
> Hi everyone,
>
> This is related to the recent thread about fp-contract and front end 
> pragma controls, but I want to generalize the discussion in terms of 
> how the target-independent codegen in the backend is implemented.
>
> Until sometime in 2017 (I think) the fast-math flags were not passed 
> through to the Selection DAG, and so the only ways to control 
> floating-point behavior were through settings in the TargetOptions or 
> by settings function attributes. Since 2017, however, the fast-math 
> flags have been attached to floating-point nodes in the selection DAG. 
> This leads to some ambiguous situations where the TargetOptions or 
> function attributes can override the absence of fast-math flags on 
> individual nodes. An example of this is the fp-contract setting. If a 
> source file is compiled with clang using the '-ffp-contract=fast' 
> setting but the file contains either "#pragma STDC FP_CONTRACT
OFF" or
> "#pragma clang fp contract(off)" the front end will generate IR 
> without the 'contract' fast-math flag set, but the X86 backend will
> generate an FMA instruction anyway.
>
> https://godbolt.org/z/dov6EcE8G
>
> This is particularly bad in the case of CUDA, because CUDA uses 
> fp-contract=fast by default. So, the user's code can explicitly say 
> "don't generate fma here" and the compiler will respond,
"meh, I think
> I will anyway."
>
> https://godbolt.org/z/c4h1nK9M3
I don’t think the argument about `-ffp-contract` is directly 
applicable to the implementation-level decision here.  For better or 
worse, we can maintain the existing `-ffp-contract` semantics on top of 
any representation, e.g. by just marking all FP instructions as 
fast-contractable and dropping all the IR that would come from pragmas.  
Your core point is that the current representation allows unnecessary 
ambiguities that lead to bugs, and that seems pretty indisputable.
> There are other cases where the backend code will check for 
> TargetOption::UnsafeFPMath for things like reassociation that can be 
> represented using fast-math flags.
>
> That brings me to the RFC part of my message. I'd like to start 
> updating the backend so that it doesn't do things like this. As a 
> general principle, I would say, "All semantics must be represented in 
> the IR and the backend must respect the IR semantics." And a 
> corollary: "Anything which can be represented at the instruction level
> must be represented at the instruction level." This corollary would 
> eliminate potential conflicts between function attributes (like 
> "unsafe-fp-math") and individual IR instructions.
It’s unclear to me whether you’re proposing this as a rule just for 
the Selection DAG or also for LLVM IR.  Selection DAG is an internal IR 
of the backends, and strengthening rules there doesn’t have very many 
downsides.  If you’re also proposing that LLVM IR should represent 
these with instruction-level rather than function-level flags, that’s 
going to be a lot more disruptive for frontends, because most frontends 
don’t need to support things like local pragmas and may not be setting 
instruction-level flags right now.  That doesn’t mean we can’t still 
pursue that, but your proposal should be clear that that’s the goal.
> As a first step toward this goal, I've prepared a patch which closes 
> the back door for fp-contract control.
>
> https://reviews.llvm.org/D112760
>
> This patch is currently incomplete, in as much as I didn't update 
> failing tests for several target architectures. I did update the X86 
> and AMDGPU tests to provide examples of how they can be made to work. 
> I will fix the rest if we decide this is the correct direction. There 
> is a failing CUDA test in the clang front end that I think will 
> require a different approach involving some driver changes to get 
> clang to generate IR for the semantics it intends rather than setting 
> an option and counting on the backend to disregard the IR.
Yeah, Clang shouldn’t be producing inconsistent IR like that.

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20211101/9d6c0ad8/attachment.html>

Thanks,
--Serge


On Fri, Oct 29, 2021 at 7:23 AM Kaylor, Andrew <andrew.kaylor at
intel.com>
wrote:
> That brings me to the RFC part of my message. I’d like to start updating
> the backend so that it doesn’t do things like this. As a general principle,
> I would say, “All semantics must be represented in the IR and the backend
> must respect the IR semantics.” And a corollary: “Anything which can be
> represented at the instruction level must be represented at the instruction
> level.” This corollary would eliminate potential conflicts between function
> attributes (like "unsafe-fp-math") and individual IR
instructions.
>
>From the viewpoint of optimization, it is important to allow internal
representation in which different instructions may have different
properties including fast-math flags. The proposed general principle looks
very attractive, as it leads to flexible representation, which is also can
be safer as properties are queried from instruction only and not a
combination instruction+function+module.

It is not mandatory to keep flags in the instruction, maybe an instruction
method that returns a set of flags would be sufficient. Such a method would
calculate the flags if they are not kept in instruction. It could help
solving problems like https://llvm.org/PR52259, as having fast-math flags
on instructions like phi or select (and probably some others) does not look
natural.

Thanks,
--Serge
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20211102/67cb8cbf/attachment.html>