llvm-project
Clang: Support minimumnum and maximumnum intrinsics
#96281
Merged
Overview
Commits
12
Changes
View On GitHub

Clang: Support minimumnum and maximumnum intrinsics #96281

wzssyqa merged 12 commits into llvm:main from wzssyqa:minimum_num_clang
wzssyqa
wzssyqa331 days ago

We just introduce llvm.minimumnum and llvm.maximumnum intrinsics support to llvm. Let's support them in Clang.

See: #93033

llvmbot llvmbot added clang
llvmbot llvmbot added clang:frontend
llvmbot llvmbot added clang:codegen
llvmbot
wzssyqa wzssyqa requested a review from philnik777 philnik777 331 days ago
wzssyqa wzssyqa requested a review from hokein hokein 331 days ago
tbaederr
tbaederr commented on 2024-06-21
Conversation is marked as resolved
Show resolved
clang/include/clang/Basic/Builtins.td
223
224def FminimumNumF16F128 : Builtin, F16F128MathTemplate {
225 let Spellings = ["__builtin_fminimum_num"];
226
 let Attributes = [FunctionWithBuiltinPrefix, NoThrow, Const, Constexpr];
tbaederr331 days ago

Where's the constexpr implementation?

philnik777
philnik777 commented on 2024-06-21
Conversation is marked as resolved
Show resolved
clang/include/clang/Basic/Builtins.td
philnik777331 days ago

Why is this a libbuiltin? IIUC this is only supposed to be an intrinsic, never alibrary function.

wzssyqa331 days ago

Oh, newer libc does have this function: https://www.gnu.org/software/libc/manual/html_node/Misc-FP-Arithmetic.html

philnik777326 days ago

In that case this should probably be a GNULibBuiltin. Or is it actually standardized?

wzssyqa326 days ago

Yes. They are in C23: https://libc.llvm.org/c23.html

wzssyqa wzssyqa force pushed from 5605426e to d364de33 326 days ago
github-actions
wzssyqa wzssyqa force pushed to 4dc133f4 219 days ago
wzssyqa wzssyqa requested a review from arsenm arsenm 219 days ago
wzssyqa wzssyqa requested a review from philnik777 philnik777 219 days ago
wzssyqa wzssyqa requested a review from tbaederr tbaederr 219 days ago
tbaederr tbaederr requested a review from AaronBallman AaronBallman 219 days ago
tbaederr
wzssyqa
tbaederr
wzssyqa
tbaederr
wzssyqa
tbaederr
arsenm
arsenm commented on 2024-10-11
Conversation is marked as resolved
Show resolved
clang/lib/AST/ExprConstant.cpp
arsenm219 days ago

Unrelated, but why is up here reproducing logic that's already in APFloat?

wzssyqa219 days ago

I have no idea. I will try to fixed in future patches.
#111991

wzssyqa218 days ago (edited 218 days ago)

I guess it was due to that the APFloat::minnum claims that it fellow IEEE-754 2019 minimumNumber semantics. Now, we have minimumnum, we should change that.
and fmin needs to follow the libc's behavior.
We need to fix them.

Conversation is marked as resolved
Show resolved
clang/test/CodeGen/math-libcalls.c
372372// HAS_MAYTRAP: declare float @llvm.experimental.constrained.minnum.f32(
373373// HAS_MAYTRAP: declare x86_fp80 @llvm.experimental.constrained.minnum.f80(
374374
375
 fmaximum_num(f,f); fmaximum_numf(f,f); fmaximum_numl(f,f);
arsenm219 days ago

Use right type and avoid the implicit casts?

Conversation is marked as resolved
Show resolved
clang/lib/Tooling/Inclusions/Stdlib/StdSymbolMap.inc
12951295SYMBOL(fminl, std::, <cmath>)
12961296SYMBOL(fminl, None, <cmath>)
12971297SYMBOL(fminl, None, <math.h>)
1298
SYMBOL(fmaximum_num, std::, <cmath>)
arsenm219 days ago

Not sure what this for, but this isn't tested?

Conversation is marked as resolved
Show resolved
clang/lib/Tooling/Inclusions/Stdlib/CSymbolMap.inc
475475SYMBOL(fmin, None, <math.h>)
476476SYMBOL(fminf, None, <math.h>)
477477SYMBOL(fminl, None, <math.h>)
478
SYMBOL(fmaximum_num, None, <math.h>)
arsenm219 days ago

Not sure what this for, but this isn't tested?

wzssyqa219 days ago

Oh, yes. It is autogenerated from https://github.com/PeterFeicht/cppreference-doc/releases.
If we need to update it, we should update cppreference.

Conversation is marked as resolved
Show resolved
clang/lib/AST/ExprConstant.cpp
15317
15318 case Builtin::BI__builtin_fmaximum_num:
15319 case Builtin::BI__builtin_fmaximum_numf:
15320
 case Builtin::BI__builtin_fmaximum_numl:
arsenm219 days ago

Missing constexpr evaluation tests

arsenm219 days ago

This doesn't have tests showing the evaluation, similar to those added for fmin/fmax in ec32386

wzssyqa219 days ago

Thanks.
@tbaederr It seems that __builtin_fmaximum_num cannot work with -fexperimental-new-constant-interpreter.
I guess I must miss something.
Any idea?

tbaederr219 days ago

Well you didn't implement that. If you add a new file anyway, you don't have to add a run line for it.

graphite-app
graphite-app commented on 2024-10-11
wzssyqa
wzssyqa
wzssyqa wzssyqa requested a review from arsenm arsenm 218 days ago
arsenm
arsenm commented on 2024-10-12
wzssyqa wzssyqa requested a review from arsenm arsenm 218 days ago
arsenm
arsenm approved these changes on 2024-10-14
arsenm
wzssyqa Clang: Support minimumnum and maximumnum intrinsics
98e909e0
wzssyqa Add ExprConstant support
f1434432
wzssyqa remove experimental_constrained_minimumnum
03982c26
wzssyqa fix code style
0329600b
wzssyqa remove from StdSymbolMap.inc
783b3961
wzssyqa add constexpr evaluation and passes a int
9938086c
wzssyqa fix typo in clang/test/Preprocessor/feature_tests.cpp
9c8d2378
wzssyqa add constexpr evaluation tests in Sema
7f60acb2
wzssyqa add invalid arguments type test
9ebfa545
wzssyqa fix typo in clang/test/Sema/constant-builtins-fminimum-num.cpp
38fcfdf5
wzssyqa clang/test/CodeGen/math-libcalls.c: use common prefix
9c821fbc
wzssyqa test constant snan
fe51305f
wzssyqa wzssyqa force pushed to fe51305f 216 days ago
wzssyqa wzssyqa merged 5bf81e53 into main 216 days ago
wzssyqa wzssyqa deleted the minimum_num_clang branch 216 days ago
wzssyqa

Login to write a write a comment.

Login via GitHub

Reviewers
Assignees
No one assigned
Labels
clang clang:frontend clang:codegen
Milestone
No milestone
Terms Of Use Privacy Policy Refund Policy Impressum