phoebewang wrote:

> It is IMO completely inacceptable to ask frontends to do that. This is the 
> kind of work backends should do.

You can't ask for freedom only when there's no cost to your end :)

> ... now I am totally confused. What are you proposing then? Just one sentence 
> above you provided a link which seems to say that `minnum_ieee` is exactly 
> like `minnum` in this PR.

I agree it's a bit confusing, because what I proposed actually is a 3-phase 
tasks. It combines both scalability to requirements and cost in the 
implementation effort. It not only beats the current solution (here current 
solution used for representing what the current revision of this patch defines 
and suggests to implement in the future) in its fixability, but also the 
benefit vs. implementation cost in every phase.

### Phase 0

Do nothing.
Implementation efforts: 0
Benefit to current solution: Save unnecessary efforts, no performance lost.

One reason I'm still against the current solution is, people tried to persuade 
me how unimportant bitcode performance is, how small the implementation effort 
takes, how little chance `nsz` been discarded etc. That's great, however, no 
one tried to give me a satisfied explanation: why do we have to do so. The 
"more expressive, more symmetric IR" is too correct to oppose. But it is 
useless, until it's proven there's a requirment behind it.

Regarding @RalfJung 's



> See https://clang.llvm.org/docs/LanguageExtensions.html, search for 
> "__builtin_elementwise_minnum" on that page: "return x or y, whichever is 
> smaller. Follows IEEE 754-2008 semantics (minNum) with +0.0>-0.0."

"__builtin_elementwise_minnum" probably unrelated here. The current solution 
makes sNaN non-deterministic. To strict follow IEEE 754-2008, we need the full 
support I'll propose in phase 2.

> And for Rust, it's simply not clear yet which semantics will be picked. 
> Having LLVM provide a reasonable matrix of options allows frontends to make 
> the choice that works best for their tradeoffs.

I don't know Rust, but according to your prior replies, I don't know why 
ordering signed 0 matters. And even it matters, any specific requirement 
`minimumnum` cannot provide.

Make sure if we do have requirement the `minimumnum` cannot provide, then let's 
move on to phase 1.

### Phase 1

To make it clear. This phase provides the capability to efficiently do zero 
ordering fmin/fmax operantion on targets only follow IEEE 754-2008 semantics 
when a language doesn't care about sNaN's determinism. This is just to mimic 
what a `minnum with +0.0>-0.0` provides but `minimumnum` not.

Implementation efforts: small
Benefit to current solution: 1) decoupling `nsz` from `minmum_ieee`; 2) sNaN 
determinism. Middle end can do optimizaitons for both sides, e.g., 
[this](https://github.com/llvm/llvm-project/pull/170181#issuecomment-3601835931)
 is not a problem then.

> * If we follow your proposal where minnum_ieee is only supported if there is 
> a corresponding hardware instruction, then this will also take additional 
> implementation complexity in each frontend, plus probably an additional 
> mechanism for LLVM to expose information about this to the frontend, because 
> it's really unreasonable to expect that frontends know on exactly which 
> targets, subtargets and types an operation is supported.

I think we may simply do it like 
[this](https://github.com/llvm/llvm-project/blob/main/clang/lib/Basic/Targets/AArch64.cpp#L932).
 Types is not a concern, if a target follows IEEE 754-2008, all its supported 
FP types should follow it. We don't need to care about those unsupported types.

> Having LLVM provide a reasonable matrix of options allows frontends to make 
> the choice that works best for their tradeoffs.

Phase 1 indeed provides one option for frontends who want to take their 
tradeoffs.

> You are also asking for code duplication and bugs, since inevitably some 
> frontends will get this check wrong as not all frontends have hardware 
> experts for all backends at hand.

Although I proposed phase 1, the requirement is too corner to be true. Almost 
all frontends just need either unodering `minnum` or `minimumnum ± nsz`. There 
does one or two frontends require it? Freedom is not free, but it's fair who 
asks the freedom pays the cost. The more freedom they require, the more cost 
they should pay. At least, the fixability of my proposal supports it. Let's 
move on to phase 2.



### Phase 2

Phase 2 is to provide the complete legalization support of `minmum_ieee`, but 
it is not another spelling of `minmum`. This is designed for frontends asking 
for deterministic sNaN behavior following IEEE 754-2008, which the current 
solution cannot provide.

Implementation efforts: medium (considering it's nearly a mirrored minimumnum 
on sNaN handing)
Benefit to current solution: Current solution cannot support it.

> Now, the costs to support minnum_ieee are:

Cost comparison makes sense only when two approaches can reach the same goal.



> Just that the new way of spelling it comes with a) lots of additional 
> implementation complexity and b) a less symmetric/understandable IR design 
> (see the table at the end of Ralf's last comment).

As explained above, it's not just a new spelling. It improves both symmetric 
and understandable. On the contrary, the current solution is not symmetric on 
the sNaN edge.



> Realistically, we'd never support a minnum_ieee intrinsic that does not have 
> full legalization support. This just goes completely against LLVM's design 
> for target-independent intrinsics.

We all have our criteria to judge whether one thing is permitted or not. For 
backend, we are keen on haggling minor performance differentiation, often deep 
diving to 1-2 cycles or 1/2-1/3 throughput differences of one instruction. So 
performance lost, no matter how corner it applies (bitcode user), how rare 
chance it happens (`nzs` discard), it is a red flag if the profit is 0. The 
question is, are we accustomed to insisting on our criteria, but hoping 
consensus can be made based on other's compromise 😅


https://github.com/llvm/llvm-project/pull/172012
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to