Skip to content

Revert "Support fp8 types in hiprtc" #3792

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.

Sign up for GitHub

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
rahulc-gh merged 1 commit into from
Mar 5, 2026
Merged

Revert "Support fp8 types in hiprtc" #3792

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions projects/clr/hipamd/include/hip/amd_detail/amd_hip_fp8.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,10 @@
#endif

// Include it explicitly for HIPRTC
#if !defined(__HIPCC_RTC__)
#include "amd_hip_bf16.h"
#include "amd_hip_mx_common.h"

#if !defined(__HIPCC_RTC__)
#include <hip/amd_detail/amd_hip_common.h>
#include <climits>

Expand All @@ -75,7 +76,6 @@
#include "amd_hip_fp16.h" // __half_raw
#include "math_fwd.h" // ocml device functions
#include "hip_assert.h" // hip assertions

#define __HIP_SCHAR_MAX SCHAR_MAX
#define __HIP_SCHAR_MIN SCHAR_MIN
#define __HIP_UCHAR_MAX UCHAR_MAX
Expand Down
2 changes: 0 additions & 2 deletions projects/clr/hipamd/include/hip/amd_detail/amd_hip_mx_common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,9 @@ SOFTWARE.
#define HIP_ENABLE_HOST_OCP_CONVERSIONS 0
#endif

#if !defined(__HIPCC_RTC__)
#include "amd_hip_ocp_types.h"
#include "amd_hip_fp16.h"
#include "amd_hip_bf16.h"
#endif

enum hipRoundMode {
hipRoundNearest = 0,
Expand Down
15 changes: 9 additions & 6 deletions projects/clr/hipamd/include/hip/amd_detail/amd_hip_ocp_fp.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,14 @@ THE SOFTWARE.

#pragma once

#if !defined(__HIPCC_RTC__)
#include "amd_hip_common.h"
#include "host_defines.h"
#include "amd_hip_ocp_types.h"
#include "amd_hip_bf16.h"
#include "amd_hip_ocp_host.hpp"
#include <hip/amd_detail/amd_hip_bf16.h>

#if !defined(__HIPCC_RTC__)
#include <hip/amd_detail/amd_hip_common.h>
#include <hip/amd_detail/host_defines.h>
#include <hip/amd_detail/amd_hip_ocp_types.h>
#include <hip/hip_fp16.h>
#include <hip/hip_bf16.h>
Comment on lines +29 to +34

Copilot AI Mar 5, 2026

Copy link

Choose a reason for hiding this comment

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

The comment indicates amd_hip_ocp_host.hpp is for host conversions, but it is now included unconditionally (including under HIPCC_RTC). If this header is host-only (or pulls in host-only dependencies), it should remain inside the !defined(HIPCC_RTC) block, or be made explicitly RTC-safe to keep include intent clear and avoid RTC build issues.

Copilot uses AI. Check for mistakes.
#include <climits>
#include <cstdio>

Expand All @@ -40,6 +41,8 @@ static_assert(sizeof(uint32_t) * CHAR_BIT == 32);
static_assert(sizeof(uint64_t) * CHAR_BIT == 64);
#endif // !defined(__HIPCC_RTC__)

#include <hip/amd_detail/amd_hip_ocp_host.hpp> // Host Conversion

// HW Detection
#if defined(__gfx950__)
Comment on lines 42 to 47

Copilot AI Mar 5, 2026

Copy link

Choose a reason for hiding this comment

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

The comment indicates amd_hip_ocp_host.hpp is for host conversions, but it is now included unconditionally (including under HIPCC_RTC). If this header is host-only (or pulls in host-only dependencies), it should remain inside the !defined(HIPCC_RTC) block, or be made explicitly RTC-safe to keep include intent clear and avoid RTC build issues.

Suggested change
#endif // !defined(__HIPCC_RTC__)
#include <hip/amd_detail/amd_hip_ocp_host.hpp> // Host Conversion
// HW Detection
#if defined(__gfx950__)
#include <hip/amd_detail/amd_hip_ocp_host.hpp> // Host Conversion
#endif // !defined(__HIPCC_RTC__)
// HW Detection
#if defined(__gfx950__)
#if defined(__gfx950__)

Copilot uses AI. Check for mistakes.
#define HIP_ENABLE_GFX950_OCP_BUILTINS 1
Expand Down
24 changes: 7 additions & 17 deletions projects/clr/hipamd/include/hip/amd_detail/amd_hip_ocp_types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,21 +24,11 @@ THE SOFTWARE.

#pragma once

#if defined(__HIPCC_RTC__)
#define __OCP_FP_HOST__
#define __OCP_FP_DEVICE__ __device__
#define __OCP_FP_HOST_DEVICE__ __OCP_FP_HOST__ __OCP_FP_DEVICE__
#define __OCP_FP_DEVICE_STATIC__ __OCP_FP_DEVICE__ static __inline__ __attribute__((always_inline))
#define __OCP_FP_HOST_DEVICE_STATIC__ __OCP_FP_HOST_DEVICE__ static
#else
#include "host_defines.h"

#define __OCP_FP_HOST__ __host__
#define __OCP_FP_DEVICE__ __device__
#define __OCP_FP_HOST_DEVICE__ __OCP_FP_HOST__ __OCP_FP_DEVICE__
#define __OCP_FP_DEVICE_STATIC__ __OCP_FP_DEVICE__ static __inline__ __attribute__((always_inline))
#define __OCP_FP_HOST_DEVICE_STATIC__ __OCP_FP_HOST_DEVICE__ static
Comment on lines 27 to 31

Copilot AI Mar 5, 2026

Copy link

Choose a reason for hiding this comment

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

The previous version included host_defines.h (in the non-RTC path) to ensure host/device are defined. With that include removed, this header may fail when included from translation units that don't already define these macros. Consider including the appropriate HIP defines header (e.g., hip/amd_detail/host_defines.h) before using host/device, or provide safe fallbacks when they are not defined.

Copilot uses AI. Check for mistakes.
#endif

static_assert(sizeof(unsigned int) == 4, "");
static_assert(sizeof(float) == 4, "");
Expand All @@ -54,17 +44,17 @@ static_assert(sizeof(_Float16) == 2, "");
// header which will act as a base abstraction, and will be maintained in the future, it makes sense
// to keep these vector types separate from existing implementations. We can add conversion
// functions in a different header using these functions.
typedef __hip_uint8_t __amd_fp8_storage_t;
typedef __hip_uint16_t __amd_fp8x2_storage_t;
typedef __hip_uint8_t __amd_fp4x2_storage_t;
typedef __hip_uint32_t __amd_fp4x8_storage_t;
typedef uint8_t __amd_fp8_storage_t;
typedef uint16_t __amd_fp8x2_storage_t;
typedef uint8_t __amd_fp4x2_storage_t;
typedef uint32_t __amd_fp4x8_storage_t;
typedef __bf16 __amd_bf16_storage_t;
typedef _Float16 __amd_fp16_storage_t;
typedef __hip_int8_t __amd_scale_t;
typedef int8_t __amd_scale_t;
Comment on lines +47 to +53

Copilot AI Mar 5, 2026

Copy link

Choose a reason for hiding this comment

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

This header now uses fixed-width integer types (uint8_t/uint16_t/uint32_t/int8_t) but does not include a header that guarantees these typedefs are available. Add an include for (or <stdint.h>) near the top of the file to avoid compilation failures depending on include order.

Copilot uses AI. Check for mistakes.

#if defined(__clang__) && (__clang_major__ > 17) && defined(__HIP__)
typedef unsigned int __attribute__((ext_vector_type(2))) __amd_uintx2_storage_t;
typedef __hip_uint8_t __attribute__((ext_vector_type(8))) __amd_fp8x8_storage_t;
typedef uint8_t __attribute__((ext_vector_type(8))) __amd_fp8x8_storage_t;

Copilot AI Mar 5, 2026

Copy link

Choose a reason for hiding this comment

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

This header now uses fixed-width integer types (uint8_t/uint16_t/uint32_t/int8_t) but does not include a header that guarantees these typedefs are available. Add an include for (or <stdint.h>) near the top of the file to avoid compilation failures depending on include order.

Copilot uses AI. Check for mistakes.
typedef __bf16 __attribute__((ext_vector_type(2))) __amd_bf16x2_storage_t;
typedef __bf16 __attribute__((ext_vector_type(8))) __amd_bf16x8_storage_t;
typedef __bf16 __attribute__((ext_vector_type(32))) __amd_bf16x32_storage_t;
Expand All @@ -75,7 +65,7 @@ typedef float __attribute__((ext_vector_type(32))) __amd_floatx32_storage_t;
typedef _Float16 __attribute__((ext_vector_type(2))) __amd_fp16x2_storage_t;
typedef _Float16 __attribute__((ext_vector_type(8))) __amd_fp16x8_storage_t;
typedef _Float16 __attribute__((ext_vector_type(32))) __amd_fp16x32_storage_t;
typedef __hip_uint32_t __attribute__((ext_vector_type(6))) __amd_fp6x32_storage_t;
typedef uint32_t __attribute__((ext_vector_type(6))) __amd_fp6x32_storage_t;

Copilot AI Mar 5, 2026

Copy link

Choose a reason for hiding this comment

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

This header now uses fixed-width integer types (uint8_t/uint16_t/uint32_t/int8_t) but does not include a header that guarantees these typedefs are available. Add an include for (or <stdint.h>) near the top of the file to avoid compilation failures depending on include order.

Copilot uses AI. Check for mistakes.
typedef short __attribute__((ext_vector_type(2))) __amd_shortx2_storage_t;
#elif defined(__GNUC__) && (__GNUC__ > 13)
/* GCC expects vector size in bytes */
Expand Down
4 changes: 0 additions & 4 deletions projects/clr/hipamd/src/hiprtc/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,6 @@ ${PROJECT_SOURCE_DIR}/include/hip/amd_detail/math_fwd.h
${PROJECT_SOURCE_DIR}/include/hip/amd_detail/hip_fp16_math_fwd.h
${PROJECT_SOURCE_DIR}/include/hip/amd_detail/amd_hip_fp16.h
${PROJECT_SOURCE_DIR}/include/hip/amd_detail/amd_math_functions.h
${PROJECT_SOURCE_DIR}/include/hip/amd_detail/amd_hip_bf16.h
${PROJECT_SOURCE_DIR}/include/hip/amd_detail/amd_hip_ocp_types.h
${PROJECT_SOURCE_DIR}/include/hip/amd_detail/amd_hip_mx_common.h
${PROJECT_SOURCE_DIR}/include/hip/amd_detail/amd_hip_fp8.h
)

# Generate required HIPRTC files.
Expand Down
1 change: 1 addition & 0 deletions projects/clr/hipamd/src/hiprtc/cmake/HIPRTC.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,3 +129,4 @@ __hipRTC_header:\n\
__hipRTC_header_size:\n\
.long __hipRTC_header_size - __hipRTC_header\n")
endmacro(generate_hiprtc_mcin)

Loading
Loading