Skip to content

[libclc] Move minmag & maxmag to the CLC library #137982

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 1, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions libclc/clc/include/clc/math/clc_maxmag.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef __CLC_MATH_CLC_MAXMAG_H__
#define __CLC_MATH_CLC_MAXMAG_H__

#define __CLC_BODY <clc/shared/binary_decl.inc>
#define __CLC_FUNCTION __clc_maxmag

#include <clc/math/gentype.inc>

#undef __CLC_FUNCTION

#endif // __CLC_MATH_CLC_MAXMAG_H__
19 changes: 19 additions & 0 deletions libclc/clc/include/clc/math/clc_minmag.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#ifndef __CLC_MATH_CLC_MINMAG_H__
#define __CLC_MATH_CLC_MINMAG_H__

#define __CLC_BODY <clc/shared/binary_decl.inc>
#define __CLC_FUNCTION __clc_minmag

#include <clc/math/gentype.inc>

#undef __CLC_FUNCTION

#endif // __CLC_MATH_CLC_MINMAG_H__
2 changes: 2 additions & 0 deletions libclc/clc/lib/generic/SOURCES
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ math/clc_log10.cl
math/clc_log1p.cl
math/clc_log2.cl
math/clc_mad.cl
math/clc_maxmag.cl
math/clc_minmag.cl
math/clc_modf.cl
math/clc_nan.cl
math/clc_native_cos.cl
Expand Down
19 changes: 19 additions & 0 deletions libclc/clc/lib/generic/math/clc_maxmag.cl
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#include <clc/clc_convert.h>
#include <clc/internal/clc.h>
#include <clc/math/clc_fabs.h>
#include <clc/math/clc_fmax.h>
#include <clc/relational/clc_isequal.h>
#include <clc/relational/clc_isgreater.h>
#include <clc/relational/clc_isnan.h>
#include <clc/relational/clc_select.h>

#define __CLC_BODY <clc_maxmag.inc>
#include <clc/math/gentype.inc>
18 changes: 18 additions & 0 deletions libclc/clc/lib/generic/math/clc_maxmag.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

_CLC_OVERLOAD _CLC_DEF __CLC_GENTYPE __clc_maxmag(__CLC_GENTYPE x,
__CLC_GENTYPE y) {
const __CLC_GENTYPE res = __clc_select(
y, x,
__CLC_CONVERT_BIT_INTN(__clc_isgreater(__clc_fabs(x), __clc_fabs(y))));
return __clc_select(
res, __clc_fmax(x, y),
__CLC_CONVERT_BIT_INTN(__clc_isnan(x) || __clc_isnan(y) ||
__clc_isequal(__clc_fabs(x), __clc_fabs(y))));
Comment on lines +11 to +17
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a lot more code than is needed. Why the isequal or conversions? This should be 1 fmax, 2 fabs + 2 select?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it does seem excessive. I don't really understand the isequal.

The conversions are necessary because select only accepts conditions of the same size as the vector, but the scalar relational functions unhelpfully return int even for double. There's no implicit conversions on vector types so we need the explicit select. This all comes out in the wash as in LLVM IR it's just i1 types fed from comparison to select anyway.

I've been wondering if we could alleviate this through extra overloads of __clc_select that accept intN conditions across the board, but that might also lead to some confusion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can just use ternary operator, I don't see what the select is adding other than complexity

}
19 changes: 19 additions & 0 deletions libclc/clc/lib/generic/math/clc_minmag.cl
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

#include <clc/clc_convert.h>
#include <clc/internal/clc.h>
#include <clc/math/clc_fabs.h>
#include <clc/math/clc_fmin.h>
#include <clc/relational/clc_isequal.h>
#include <clc/relational/clc_isless.h>
#include <clc/relational/clc_isnan.h>
#include <clc/relational/clc_select.h>

#define __CLC_BODY <clc_minmag.inc>
#include <clc/math/gentype.inc>
17 changes: 17 additions & 0 deletions libclc/clc/lib/generic/math/clc_minmag.inc
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//

_CLC_OVERLOAD _CLC_DEF __CLC_GENTYPE __clc_minmag(__CLC_GENTYPE x,
__CLC_GENTYPE y) {
const __CLC_GENTYPE res = __clc_select(
y, x, __CLC_CONVERT_BIT_INTN(__clc_isless(__clc_fabs(x), __clc_fabs(y))));
return __clc_select(
res, __clc_fmin(x, y),
__CLC_CONVERT_BIT_INTN(__clc_isnan(x) || __clc_isnan(y) ||
__clc_isequal(__clc_fabs(x), __clc_fabs(y))));
}
5 changes: 3 additions & 2 deletions libclc/generic/lib/math/maxmag.cl
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
//===----------------------------------------------------------------------===//

#include <clc/clc.h>
#include <clc/utils.h>
#include <clc/math/clc_maxmag.h>

#define __CLC_BODY <maxmag.inc>
#define FUNCTION maxmag
#define __CLC_BODY <clc/shared/binary_def.inc>
#include <clc/math/gentype.inc>
22 changes: 0 additions & 22 deletions libclc/generic/lib/math/maxmag.inc

This file was deleted.

5 changes: 3 additions & 2 deletions libclc/generic/lib/math/minmag.cl
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
//===----------------------------------------------------------------------===//

#include <clc/clc.h>
#include <clc/utils.h>
#include <clc/math/clc_minmag.h>

#define __CLC_BODY <minmag.inc>
#define FUNCTION minmag
#define __CLC_BODY <clc/shared/binary_def.inc>
#include <clc/math/gentype.inc>
30 changes: 0 additions & 30 deletions libclc/generic/lib/math/minmag.inc

This file was deleted.

Loading