search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.89k stars 11.51k forks source link

vec_absd should build with -mpower9-vector as it does with -mcpu=power9 #84703

Open lu-zero opened 6 months ago

lu-zero commented 6 months ago

Right now with both clang-16 and clang-17 it triggers a:

fatal error: error in backend: Cannot select: intrinsic %llvm.ppc.altivec.vabsdub
llvmbot commented 6 months ago

@llvm/issue-subscribers-backend-powerpc

Author: Luca Barbato (lu-zero)

Right now with both clang-16 and clang-17 it triggers a: ``` fatal error: error in backend: Cannot select: intrinsic %llvm.ppc.altivec.vabsdub ```
chenzheng1030 commented 6 months ago

hmm, actually -mpower9-vector is a switch for instructions that start with xv/xs/xx(vector-scalar instructions) on pwr9. Instruction starting with v like vabsdub is guarded by altivec feature bit. So this is some kind of by design. Unfortunately there is no -mpower9-altivec option in clang for now. So can only enable this builtin with -mcpu=power9.

chenzheng1030 commented 6 months ago

There is doc about this builtin usage in https://www.ibm.com/docs/en/openxl-c-and-cpp-aix/17.1.2?topic=functions-vec-absd

lu-zero commented 6 months ago

gcc does allow vec_absd with -mpower9-vector and the intrinsic is guarded by __POWER9_VECTOR__ in altivec.h thus my confusion.

chenzheng1030 commented 6 months ago

gcc does allow vec_absd with -mpower9-vector and the intrinsic is guarded by __POWER9_VECTOR__ in altivec.h thus my confusion.

OK. That's a good reason we should check llvm. llvm should be compatible with gcc for this option and its behavior.

bzEq commented 5 months ago

Hi @lu-zero Which gcc version are you using? How is -mpower9-vector documented in GCC?

lu-zero commented 5 months ago

gcc-13, the manpage mentions it in passing as:

If you use the ISA 3.0 instruction set (-mpower9-vector or -mcpu=power9) ...

The part that is annoying with clang is that __POWER9_VECTOR__ is the only guard in the clang-provided altivec.h so one would be none-the-wiser when looking at the instructions there and papr also does not differentiate: ISA 3.0 the only discriminant documented.

bzEq commented 5 months ago

I've posted an RFC PR https://github.com/llvm/llvm-project/pull/86905. @lu-zero I've checked the manual of the latest GCC manual which doesn't list -mpower9-vector like -mcpu any more. I think clang should follow the latest gcc behavior, i.e., I suggest you use -mcpu=pwr9 rather than -mpower9-vector. Some GCC PPC port developers told me, this might be a debug option which is not for long-term uses.

lu-zero commented 5 months ago

pwr9 or power9 ? Right now I set the CFLAGS for the code using vec_absd to -mcpu=power9 in dav1d, and indeed would be nice if the -mcpu does not need to be changed again...

(and yes, it is already annoying to have -Ctarget-cpu to support pwr9 only in rustc).

bzEq commented 5 months ago

pwr9 or power9

I think clang should support both.

chenzheng1030 commented 5 months ago

I agreed with @bzEq 's conclusion that if GCC is going to drop this option, Clang should not try to support this option either. To me, the best solution for your case is to use -mcpu=power9(for GCC) or -mcpu=[pwr9|power9](for Clang). If you upgrade your GCC compiler in future, the option might be not supported any longer, right?

lu-zero commented 5 months ago

The option cannot disappear overnight if they do not want to make even less developers interested in supporting power.

In general you want to build with -m only the bit of code of interest and let the user pass -mcpu/-march.

What would happen if somebody has a Power 11 and wants to pass -mcpu=pwr11 and the build system ends up adding -mcpu=pwr9 ?

I'd rather have -maltivec -mvsx -mpower9-vector.

bzEq commented 5 months ago

What would happen if somebody has a Power 11 and wants to pass -mcpu=pwr11 and the build system ends up adding -mcpu=pwr9 ?

Can you elaborate? To me, it should be an issue inside the build system.

bzEq commented 5 months ago

The option cannot disappear overnight if they do not want to make even less developers interested in supporting power.

I think it's common to deprecate some options when upgrading compiler. These options in the context are intended originally for debug purpose as far as I know.

lu-zero commented 5 months ago

What would happen if somebody has a Power 11 and wants to pass -mcpu=pwr11 and the build system ends up adding -mcpu=pwr9 ?

Can you elaborate? To me, it should be an issue inside the build system.

Most build systems take additional cflags.

Having multiple -mcpu is confusing and you have to make sure to remember right-most-wins.

Having -m{isa-extension} like -mvsx or -mavx512ifma let you state clearly the intent, thus why I'm puzzled that I have to pass -mcpu