[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[kaushikp at gcc dot gnu.org]

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

--- Comment #27 from kaushikp at gcc dot gnu.org ---
I have verified the backported patch to GCC-7 and it fixes the issues
I had observed earlier.

Thanks again Peter for this!

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[bergner at gcc dot gnu.org]

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

Peter Bergner  changed:

   What|Removed |Added

 Status|RESOLVED|CLOSED

--- Comment #26 from Peter Bergner  ---
Backport to GCC 7 committed.  Closing as fixed.

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[bergner at gcc dot gnu.org]

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

--- Comment #25 from Peter Bergner  ---
Author: bergner
Date: Fri Mar 23 17:48:58 2018
New Revision: 258819

URL: https://gcc.gnu.org/viewcvs?rev=258819=gcc=rev
Log:
Backport from mainline
2018-03-20  Peter Bergner  

PR target/83789
* config/rs6000/altivec.md (altivec_lvx__2op): Delete
define_insn.
(altivec_lvx__1op): Likewise.
(altivec_stvx__2op): Likewise.
(altivec_stvx__1op): Likewise.
(altivec_lvx_): New define_expand.
(altivec_stvx_): Likewise.
(altivec_lvx__2op_): New define_insn.
(altivec_lvx__1op_): Likewise.
(altivec_stvx__2op_): Likewise.
(altivec_stvx__1op_): Likewise.
* config/rs6000/rs6000.c (altivec_expand_lv_builtin): Likewise.
(altivec_expand_stv_builtin): Likewise.
(altivec_expand_builtin): Likewise.
* config/rs6000/vector.md: Likewise.

Modified:
branches/gcc-7-branch/gcc/ChangeLog
branches/gcc-7-branch/gcc/config/rs6000/altivec.md
branches/gcc-7-branch/gcc/config/rs6000/rs6000.c
branches/gcc-7-branch/gcc/config/rs6000/vector.md

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[bergner at gcc dot gnu.org]

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

--- Comment #24 from Peter Bergner  ---
(In reply to Peter Bergner from comment #23)
> This regtested fine on BE for me with no regressions.  My LE
> bootstrap/regtest is still running.

My LE bootstrap and regtesting were clean too.  Just waiting on Kaushik to
verify the patch fixes the ICE on GCC 7.

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[bergner at gcc dot gnu.org]

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

--- Comment #23 from Peter Bergner  ---
Created attachment 43728
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=43728=edit
Backport of trunk patch to GCC 7

Kaushik, can you verify the attached backported patch fixes the ICE on GCC 7?

This regtested fine on BE for me with no regressions.  My LE bootstrap/regtest
is still running.

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[bergner at gcc dot gnu.org]

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

Peter Bergner  changed:

   What|Removed |Added

   Target Milestone|--- |7.4

--- Comment #22 from Peter Bergner  ---
(In reply to Kaushik.Phatak from comment #21)
> >> Kaushik, remind me, you're seeing the same ICE in GCC 7 as well
> Yes, this does fail in gcc-7.x.

Ok, then I'll work on backporting the patch.  Thanks.

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[Kaushik.Phatak at kpit dot com]

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

--- Comment #21 from Kaushik.Phatak at kpit dot com ---
Hi Peter,
Thanks for your work on this.

>> Kaushik, remind me, you're seeing the same ICE in GCC 7 as well
Yes, this does fail in gcc-7.x.

>> so we need a backport of the patch committed to trunk?
If a backport to GCC 7 is feasible, then that would be very much appreciated.
I tried patching parts of the file as-is, but it fails while patching the hunk
to the file, rs6000-p8swap.c
This file (rs6000-p8swap.c) does not exist in gcc-7.x and was introduced in 8,
I believe.

Thanks & Best Regards,
Kaushik M. Phatak

-Original Message-
From: bergner at gcc dot gnu.org [mailto:gcc-bugzi...@gcc.gnu.org] 
Sent: Tuesday, March 20, 2018 11:02 PM
To: Kaushik Phatak <kaushik.pha...@kpit.com>
Subject: [Bug target/83789] __builtin_altivec_lvx fails for powerpc for
altivec-4.c

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

--- Comment #20 from Peter Bergner  --- Kaushik,
remind me, you're seeing the same ICE in GCC 7 as well, so we need a backport
of the patch committed to trunk?

--
You are receiving this mail because:
You are on the CC list for the bug.
You reported the bug.

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[bergner at gcc dot gnu.org]

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

--- Comment #20 from Peter Bergner  ---
Kaushik, remind me, you're seeing the same ICE in GCC 7 as well, so we need a
backport of the patch committed to trunk?

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[bergner at gcc dot gnu.org]

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

Peter Bergner  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 Resolution|--- |FIXED

--- Comment #19 from Peter Bergner  ---
Fixed on trunk.

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[bergner at gcc dot gnu.org]

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

--- Comment #18 from Peter Bergner  ---
Author: bergner
Date: Tue Mar 20 17:25:09 2018
New Revision: 258688

URL: https://gcc.gnu.org/viewcvs?rev=258688=gcc=rev
Log:
PR target/83789
* config/rs6000/altivec.md (altivec_lvx__2op): Delete
define_insn.
(altivec_lvx__1op): Likewise.
(altivec_stvx__2op): Likewise.
(altivec_stvx__1op): Likewise.
(altivec_lvx_): New define_expand.
(altivec_stvx_): Likewise.
(altivec_lvx__2op_): New define_insn.
(altivec_lvx__1op_): Likewise.
(altivec_stvx__2op_): Likewise.
(altivec_stvx__1op_): Likewise.
* config/rs6000/rs6000-p8swap.c (rs6000_gen_stvx): Use new expanders.
(rs6000_gen_lvx): Likewise.
* config/rs6000/rs6000.c (altivec_expand_lv_builtin): Likewise.
(altivec_expand_stv_builtin): Likewise.
(altivec_expand_builtin): Likewise.
* config/rs6000/vector.md: Likewise.

Modified:
trunk/gcc/ChangeLog
trunk/gcc/config/rs6000/altivec.md
trunk/gcc/config/rs6000/rs6000-p8swap.c
trunk/gcc/config/rs6000/rs6000.c
trunk/gcc/config/rs6000/vector.md

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[bergner at gcc dot gnu.org]

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

Peter Bergner  changed:

   What|Removed |Added

  Attachment #43617|0   |1
is obsolete||

--- Comment #17 from Peter Bergner  ---
Created attachment 43621
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=43621=edit
Version 3 of the patch

Final version of the patch that bootstraps and regtests with no regressions on
both BE and LE.  The previous patch had some testsuite regressions on LE.

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[kaushikp at gcc dot gnu.org]

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

kaushikp at gcc dot gnu.org changed:

   What|Removed |Added

 CC||kaushikp at gcc dot gnu.org

--- Comment #16 from kaushikp at gcc dot gnu.org ---
Thanks for the patch Peter.
gcc builds without errors on trunk with this patch.

I will let you know how my tests go.

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[bergner at gcc dot gnu.org]

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

Peter Bergner  changed:

   What|Removed |Added

  Attachment #43611|0   |1
is obsolete||

--- Comment #15 from Peter Bergner  ---
Created attachment 43617
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=43617=edit
Yet another updated patch

Sorry, the previous patch had a typo that only seemed to show up on my LE
testing.  Please test this patch to see if it fixes your ICE.

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[bergner at gcc dot gnu.org]

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

Peter Bergner  changed:

   What|Removed |Added

  Attachment #43419|0   |1
is obsolete||

--- Comment #14 from Peter Bergner  ---
Created attachment 43611
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=43611=edit
Updated patch

Here's the patch I'm testing now that removes the duplicate patterns and
simplifies the callers of the patterns.

Kaushik, can you test that this fixes the ICE you're seeing, since Segher and I
cannot recreate the ICE you're seeing?

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[bergner at gcc dot gnu.org]

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

--- Comment #13 from Peter Bergner  ---
Ok, I have a patch that eliminates the duplicate patterns and greatly
simplifies the code that calls these patterns.  I'm testing it now.

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[kaushik.phatak at kpitcummins dot com]

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

--- Comment #12 from Kaushikp  ---
>> we're getting duplicate named patterns.
Yes, it does generate multiple patters for the concerned built-ins causing
toolchain build failure.
I think it's trying to force the same pattern for 32-bit which exist for 64-bit
causing redefinition error.

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[bergner at gcc dot gnu.org]

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

--- Comment #11 from Peter Bergner  ---
Ah, testing my patch, we're getting duplicate named patterns.  I'll have to
think about this.

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[bergner at gcc dot gnu.org]

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

Peter Bergner  changed:

   What|Removed |Added

  Attachment #43417|0   |1
is obsolete||

--- Comment #10 from Peter Bergner  ---
Created attachment 43419
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=43419=edit
Updated patch without debug cruft.  No other changes.

Oops, I didn't mean to leave in stuff to make my make check less noisy.  Here's
an updated patch.

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[bergner at gcc dot gnu.org]

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

--- Comment #9 from Peter Bergner  ---
Created attachment 43417
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=43417=edit
Proposed patch to merge altivec patterns.

Hi Kaushik,

Can you see if the following unetsted patch fixes your ICE?  In the end, it's
cleaner to just combine the two patterns.

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[bergner at gcc dot gnu.org]

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

--- Comment #8 from Peter Bergner  ---
(In reply to Segher Boessenkool from comment #7)
> See https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00031.html -- I think
> Kaushik
> is still looking at this?

Ah, ok.  I do agree with your comment to use the *_2op_si pattern.

Do you agree we could combine these patterns?  If so, I could do that as a
followon cleanup patch.

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[segher at gcc dot gnu.org]

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

--- Comment #7 from Segher Boessenkool  ---
See https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00031.html -- I think Kaushik
is still looking at this?

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[bergner at gcc dot gnu.org]

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

Peter Bergner  changed:

   What|Removed |Added

 CC||bergner at gcc dot gnu.org

--- Comment #6 from Peter Bergner  ---
Segher, I can have a look at this if you aren't already.  I can see how of the
builtin only uses the altivec_lxv__2op patterm, how it would fail on a
-m32 compile, since that pattern is disabled and you're supposed to use
altivec_lxv__2op_si.

That said, why do we have two patterns at all?  It seems like they should be
able to be combined fairly easily using the P iterator:

; The next two patterns embody what lvx should usually look like.
(define_insn "altivec_lvx__2op"
  [(set (match_operand:VM2 0 "register_operand" "=v")
(mem:VM2 (and:DI (plus:DI (match_operand:DI 1 "register_operand" "b")
  (match_operand:DI 2 "register_operand" "r"))
 (const_int -16]
  "TARGET_ALTIVEC && TARGET_64BIT"
  "lvx %0,%1,%2"
  [(set_attr "type" "vecload")])

; 32-bit versions of the above.
(define_insn "altivec_lvx__2op_si"
  [(set (match_operand:VM2 0 "register_operand" "=v")
(mem:VM2 (and:SI (plus:SI (match_operand:SI 1 "register_operand" "b")
  (match_operand:SI 2 "register_operand" "r"))
 (const_int -16]
  "TARGET_ALTIVEC && TARGET_32BIT"
  "lvx %0,%1,%2"
  [(set_attr "type" "vecload")])

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[kaushik.phatak at kpitcummins dot com]

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

--- Comment #5 from Kaushikp  ---
>> Kaushik: is this fixed with r256762?
No. The testcase still fails with internal compiler error: Segmentation fault.
This revision (r256762) dated 16th Jan 2018 still calls
'CODE_FOR_altivec_lvx_v4si_2op' insn
for ALTIVEC_BUILTIN_LVX. This is only defined for 64-bit target, hence it
fails.

The latest revision r257477 dated today, 8th Feb 2018 also suffers from the
same issue.

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[segher at gcc dot gnu.org]

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

--- Comment #4 from Segher Boessenkool  ---
Kaushik: is this fixed with r256762?

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[segher at gcc dot gnu.org]

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

--- Comment #3 from Segher Boessenkool  ---
Does not reproduce with powerpc64-linux-gcc -m32 (not on trunk and
not on current 7).

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[segher at gcc dot gnu.org]

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

--- Comment #2 from Segher Boessenkool  ---
(In reply to Kaushikp from comment #0)

> Should we modify the testcase so it runs only for 64-bit targets, 
> or should this builtin work for 32-bit targets as well?

It shouldn't ICE for 32-bit either, whether the testcase really applies
to 32-bit or not (I haven't looked).

[Bug target/83789] __builtin_altivec_lvx fails for powerpc for altivec-4.c

[kaushik.phatak at kpitcummins dot com]

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

--- Comment #1 from Kaushikp  ---
Just some additional info on this issue, 
The powerpc-elf-gcc did not have any issues compiling this testcase with
identical options on gcc-7.2.0; 
powerpc-linux-gcc generated segmentation fault; 
powerpc64-linux-gcc passed without errors or warnings.