PPC64 big-endian support #398
Conversation
awilfox
left a comment
awilfox
left a comment
There was a problem hiding this comment.
Few nits I found while reviewing the changes.
Makefile
Outdated
| ifneq (,$(findstring POWER9,$(POWER9_M))) | ||
| CFLAGS += -mpower9-vector -DGGML_BIG_ENDIAN | ||
| CXXFLAGS += -std=c++23 -DGGML_BIG_ENDIAN | ||
| endif |
There was a problem hiding this comment.
It seems that this port will be big-endian even if run on non-Power9 systems (consider P8, P10, or any other generation), so I think this is more correct:
| ifneq (,$(findstring POWER9,$(POWER9_M))) | |
| CFLAGS += -mpower9-vector -DGGML_BIG_ENDIAN | |
| CXXFLAGS += -std=c++23 -DGGML_BIG_ENDIAN | |
| endif | |
| ifneq (,$(findstring POWER9,$(POWER9_M))) | |
| CFLAGS += -mpower9-vector | |
| endif | |
| CFLAGS += -DGGML_BIG_ENDIAN | |
| CXXFLAGS += -std=c++23 -DGGML_BIG_ENDIAN |
ggml.c
Outdated
| case GGML_TYPE_F16: { | ||
| unsigned short * toswap = (unsigned short *)(node->src1->data); | ||
| for (int k = 0; k < (ggml_nbytes(node->src1) / sizeof(unsigned short)); k++) { | ||
| toswap[k] = bswap_16(toswap[k]); |
There was a problem hiding this comment.
Any reason for using std::byteswap in whisper.cpp but not here?
whisper.cpp
Outdated
| static void read_safe(std::ifstream& fin, T& dest) { | ||
| fin.read((char*)& dest, sizeof(T)); | ||
| #if defined(GGML_BIG_ENDIAN) | ||
| if constexpr (std::endian::native == std::endian::big) { |
There was a problem hiding this comment.
I realise the reason for the guard is because <bit> is guarded by it, but this seems wrong; if that's defined, it will always be BE.
What you probably want is something like #if __cplusplus >= 202110L (the revision of C++2x that added std::byteswap).
|
This is ready to merge now. @awilfox, all your comments on the draft are addressed, thanks. Maybe |
* ggml : set cache line size to 128 on POWER9 * whisper : add PPC64 big endian support
* ggml : set cache line size to 128 on POWER9 * whisper : add PPC64 big endian support
* ggml : set cache line size to 128 on POWER9 * whisper : add PPC64 big endian support
* ggml : set cache line size to 128 on POWER9 * whisper : add PPC64 big endian support
* ggml : set cache line size to 128 on POWER9 * whisper : add PPC64 big endian support
3 participants
No description provided.