diff mbox

[rs6000] Fix pasto resulting in wrong instruction from builtins for lvxl

Message ID 1455651463.13491.10.camel@gnopaine
State New
Headers show

Commit Message

Bill Schmidt Feb. 16, 2016, 7:37 p.m. UTC 
Hi,

During the little-endian vector modification work in 2014, I
accidentally introduced an error that Uli Weigand noticed this week.
This results in wrong code being generated for the
__builtin_altivec_lvxl and vec_lvxl interfaces; an "lvx" instruction is
generated instead of an "lvxl" instruction.  Now, this is only a
performance issue, since the error just means a cache hint is not being
generated.  However, it needs to be corrected, as below.

This brings up a point that, though we have many test cases for the
altivec built-ins, they only test that well-formed built-ins are
accepted.  None of them test the actual code generation.  I don't intend
to fix that here, but at some point we should do better with this.  We
should definitely do better with future built-ins that we add.

Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no
regressions.  I hand-checked some of the existing test cases that invoke
__builtin_altivec_lvxl and vec_lvxl to verify correct code gen.  Is this
okay for trunk?  I would also like to backport this to GCC 5 and 4.9 if
that's acceptable.

Thanks,
Bill


2016-02-16  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	* config/rs6000/altivec.md (*altivec_lvxl_<mode>_internal): Output
	correct instruction.

Comments

Bill Schmidt Feb. 16, 2016, 8:41 p.m. UTC | #1 
On Tue, 2016-02-16 at 11:40 -0800, David Edelsohn wrote:
> This is okay, but how about starting with a testcase for this? 

That's fine.  I'll make it generic enough that we can add to it later,
then.

Bill
> 
> Thanks David
> 
> On Feb 16, 2016 11:37 AM, "Bill Schmidt" <wschmidt@linux.vnet.ibm.com>
> wrote:
>         Hi,
>         
>         During the little-endian vector modification work in 2014, I
>         accidentally introduced an error that Uli Weigand noticed this
>         week.
>         This results in wrong code being generated for the
>         __builtin_altivec_lvxl and vec_lvxl interfaces; an "lvx"
>         instruction is
>         generated instead of an "lvxl" instruction.  Now, this is only
>         a
>         performance issue, since the error just means a cache hint is
>         not being
>         generated.  However, it needs to be corrected, as below.
>         
>         This brings up a point that, though we have many test cases
>         for the
>         altivec built-ins, they only test that well-formed built-ins
>         are
>         accepted.  None of them test the actual code generation.  I
>         don't intend
>         to fix that here, but at some point we should do better with
>         this.  We
>         should definitely do better with future built-ins that we add.
>         
>         Bootstrapped and tested on powerpc64le-unknown-linux-gnu with
>         no
>         regressions.  I hand-checked some of the existing test cases
>         that invoke
>         __builtin_altivec_lvxl and vec_lvxl to verify correct code
>         gen.  Is this
>         okay for trunk?  I would also like to backport this to GCC 5
>         and 4.9 if
>         that's acceptable.
>         
>         Thanks,
>         Bill
>         
>         
>         2016-02-16  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>         
>                 * config/rs6000/altivec.md
>         (*altivec_lvxl_<mode>_internal): Output
>                 correct instruction.
>         
>         
>         Index: gcc/config/rs6000/altivec.md
>         ===================================================================
>         --- gcc/config/rs6000/altivec.md        (revision 233466)
>         +++ gcc/config/rs6000/altivec.md        (working copy)
>         @@ -2511,7 +2511,7 @@
>                   (match_operand:VM2 1 "memory_operand" "Z"))
>               (unspec [(const_int 0)] UNSPEC_SET_VSCR)])]
>            "TARGET_ALTIVEC"
>         -  "lvx %0,%y1"
>         +  "lvxl %0,%y1"
>            [(set_attr "type" "vecload")])
>         
>          (define_expand "altivec_lvx_<mode>"
>         
>
diff mbox

Patch

Index: gcc/config/rs6000/altivec.md
===================================================================
--- gcc/config/rs6000/altivec.md	(revision 233466)
+++ gcc/config/rs6000/altivec.md	(working copy)
@@ -2511,7 +2511,7 @@ 
 	  (match_operand:VM2 1 "memory_operand" "Z"))
      (unspec [(const_int 0)] UNSPEC_SET_VSCR)])]
   "TARGET_ALTIVEC"
-  "lvx %0,%y1"
+  "lvxl %0,%y1"
   [(set_attr "type" "vecload")])
 
 (define_expand "altivec_lvx_<mode>"