PPC64 builtin vec_rlnm() argument order is wrong

Message ID 59f645d57c7f5c3562209627204e407f58c50210.camel@us.ibm.com
State New
Headers show
Series
  • PPC64 builtin vec_rlnm() argument order is wrong
Related show

Commit Message

Carl Love Feb. 19, 2020, 4:16 p.m.
GCC maintainers:

The implemented argument order for the vec_rlnm() builtin for PPC64 is
wrong.  The following bugzilla was created for the issue:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93819

I included a patch to fix the issue in the patch.  Per the request from
Martin Liska I am posting the patch as well to the GCC mailing list.  

The patch was tested on

 powerpc64le-unknown-linux-gnu (Power 9 LE)

using the GCC mainline with the last commit:

r280156 | jsm28 | 2020-01-10 18:16:54 -0600 (Fri, 10 Jan 2020) | 4
lines

Add README.MOVED_TO_GIT.

	* README.MOVED_TO_GIT: New file.

If someone can verify the issue and the fix, it would be appreciated. 
Please update the bugzilla with any feedback on the issue.  Thanks.

                Carl Love

-------------------------------------------------

vec_rlnm fix to make builtin work according to ABI

---
 gcc/config/rs6000/altivec.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.17.1

Comments

Segher Boessenkool March 25, 2020, 8:40 a.m. | #1
Hi Carl,

Sorry I didn't reply to this patch yet :-(

On Wed, Feb 19, 2020 at 08:16:13AM -0800, Carl Love wrote:
> The implemented argument order for the vec_rlnm() builtin for PPC64 is

> wrong.  The following bugzilla was created for the issue:

> 

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93819


> I included a patch to fix the issue in the patch.  Per the request from

> Martin Liska I am posting the patch as well to the GCC mailing list.  


Please use a changelog entry like

	* config/rs6000/altivec.h (vec_rlmn): Fix swapped arguments.

> If someone can verify the issue and the fix, it would be appreciated. 


It looks correct, yes.

Is there some test that could catch this?  And similar cases (*are* there
any similar builtins / macros / etc.?)

Okay for trunk either way.  Thanks!  Also okay for backporting, after
letting it simmer for a bit.


Segher


> vec_rlnm fix to make builtin work according to ABI

> 

> ---

>  gcc/config/rs6000/altivec.h | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/gcc/config/rs6000/altivec.h b/gcc/config/rs6000/altivec.h

> index e0b6547c6..5f1f59244 100644

> --- a/gcc/config/rs6000/altivec.h

> +++ b/gcc/config/rs6000/altivec.h

> @@ -182,7 +182,7 @@

>  #define vec_recipdiv __builtin_vec_recipdiv

>  #define vec_rlmi __builtin_vec_rlmi

>  #define vec_vrlnm __builtin_vec_rlnm

> -#define vec_rlnm(a,b,c) (__builtin_vec_rlnm((a),((b)<<8)|(c)))

> +#define vec_rlnm(a,b,c) (__builtin_vec_rlnm((a),((c)<<8)|(b)))

>  #define vec_rsqrt __builtin_vec_rsqrt

>  #define vec_rsqrte __builtin_vec_rsqrte

>  #define vec_signed __builtin_vec_vsigned

Patch

diff --git a/gcc/config/rs6000/altivec.h b/gcc/config/rs6000/altivec.h
index e0b6547c6..5f1f59244 100644
--- a/gcc/config/rs6000/altivec.h
+++ b/gcc/config/rs6000/altivec.h
@@ -182,7 +182,7 @@ 
 #define vec_recipdiv __builtin_vec_recipdiv
 #define vec_rlmi __builtin_vec_rlmi
 #define vec_vrlnm __builtin_vec_rlnm
-#define vec_rlnm(a,b,c) (__builtin_vec_rlnm((a),((b)<<8)|(c)))
+#define vec_rlnm(a,b,c) (__builtin_vec_rlnm((a),((c)<<8)|(b)))
 #define vec_rsqrt __builtin_vec_rsqrt
 #define vec_rsqrte __builtin_vec_rsqrte
 #define vec_signed __builtin_vec_vsigned