diff mbox

[gomp4] Avoiding predication for certain blocks

Message ID 556C2F84.80207@mentor.com
State New
Headers show

Commit Message

Tom de Vries June 1, 2015, 10:10 a.m. UTC 
On 29/05/15 18:23, Bernd Schmidt wrote:
> When predicating the code for OpenACC, we should avoid the entry block
> in an offloaded region, which contains setup code that should be run in
> every thread. The following patch adds a new marker statement that is
> used to identify this block. Currently, predication doesn't happen
> anyway due to an oversight in the algorithm, but I'll be fixing that in
> a followup patch.
>
> Committed on gomp-4_0-branch.
>

Hi Bernd,

this causes an ICE in kernels-eternal.c.

The GIMPLE_OMP_ENTRY_END is removed the first time (!ssa) we call 
expand_omp_target. The second time we call expand_omp_target (ssa), it's 
no longer there, and we call 'gimple_code (NULL)' here:
...
   basic_block entry_succ_bb = single_succ (entry_bb);
   gsi = gsi_last_bb (entry_succ_bb);
   if (gimple_code (gsi_stmt (gsi)) == GIMPLE_OMP_ENTRY_END)
     gsi_remove (&gsi, true);
...

This patch fixes that by not attempting to remove GIMPLE_OMP_ENTRY_END 
when in ssa.

Committed to gomp-4_0-branch.

Thanks,
- Tom

Comments

Thomas Schwinge June 1, 2015, 11:57 a.m. UTC | #1 
Hi!

On Mon, 1 Jun 2015 12:10:12 +0200, Tom de Vries <Tom_deVries@mentor.com> wrote:
> On 29/05/15 18:23, Bernd Schmidt wrote:
> > When predicating the code for OpenACC, we should avoid the entry block
> > in an offloaded region, which contains setup code that should be run in
> > every thread. The following patch adds a new marker statement that is
> > used to identify this block. Currently, predication doesn't happen
> > anyway due to an oversight in the algorithm, but I'll be fixing that in
> > a followup patch.
> >
> > Committed on gomp-4_0-branch.

> this causes an ICE in kernels-eternal.c.

(Belatedly) confirmed:

    [-PASS:-]{+FAIL: c-c++-common/goacc/kernels-eternal.c (internal compiler error)+}
    {+FAIL:+} c-c++-common/goacc/kernels-eternal.c (test for excess errors)

    [-PASS:-]{+FAIL: c-c++-common/goacc/kernels-eternal.c  -std=c++11 (internal compiler error)+}
    {+FAIL:+} c-c++-common/goacc/kernels-eternal.c  -std=c++11 (test for excess errors)
    [-PASS:-]{+FAIL: c-c++-common/goacc/kernels-eternal.c  -std=c++14 (internal compiler error)+}
    {+FAIL:+} c-c++-common/goacc/kernels-eternal.c  -std=c++14 (test for excess errors)
    [-PASS:-]{+FAIL: c-c++-common/goacc/kernels-eternal.c  -std=c++98 (internal compiler error)+}
    {+FAIL:+} c-c++-common/goacc/kernels-eternal.c  -std=c++98 (test for excess errors)

> The GIMPLE_OMP_ENTRY_END is removed the first time (!ssa) we call 
> expand_omp_target. The second time we call expand_omp_target (ssa), it's 
> no longer there, and we call 'gimple_code (NULL)' here:
> ...
>    basic_block entry_succ_bb = single_succ (entry_bb);
>    gsi = gsi_last_bb (entry_succ_bb);
>    if (gimple_code (gsi_stmt (gsi)) == GIMPLE_OMP_ENTRY_END)
>      gsi_remove (&gsi, true);
> ...
> 
> This patch fixes that by not attempting to remove GIMPLE_OMP_ENTRY_END 
> when in ssa.
> 
> Committed to gomp-4_0-branch.

I also observed -- and still do observe -- ICEs as follows.  Tom, as
you've fixed (thanks!) this already for
c-c++-common/goacc/kernels-eternal.c, would you mind fixing it for these,
too:

    [-PASS:-]{+FAIL: c-c++-common/goacc-gomp/nesting-1.c (internal compiler error)+}
    {+FAIL:+} c-c++-common/goacc-gomp/nesting-1.c (test for excess errors)

    [-PASS:-]{+FAIL: c-c++-common/gomp/nesting-1.c (internal compiler error)+}
    {+FAIL:+} c-c++-common/gomp/nesting-1.c (test for excess errors)

    [-PASS:-]{+FAIL: c-c++-common/goacc-gomp/nesting-1.c  -std=c++11 (internal compiler error)+}
    {+FAIL:+} c-c++-common/goacc-gomp/nesting-1.c  -std=c++11 (test for excess errors)
    [-PASS:-]{+FAIL: c-c++-common/goacc-gomp/nesting-1.c  -std=c++14 (internal compiler error)+}
    {+FAIL:+} c-c++-common/goacc-gomp/nesting-1.c  -std=c++14 (test for excess errors)
    [-PASS:-]{+FAIL: c-c++-common/goacc-gomp/nesting-1.c  -std=c++98 (internal compiler error)+}
    {+FAIL:+} c-c++-common/goacc-gomp/nesting-1.c  -std=c++98 (test for excess errors)

    [-PASS:-]{+FAIL: c-c++-common/gomp/nesting-1.c  -std=c++11 (internal compiler error)+}
    {+FAIL:+} c-c++-common/gomp/nesting-1.c  -std=c++11 (test for excess errors)
    [-PASS:-]{+FAIL: c-c++-common/gomp/nesting-1.c  -std=c++14 (internal compiler error)+}
    {+FAIL:+} c-c++-common/gomp/nesting-1.c  -std=c++14 (test for excess errors)
    [-PASS:-]{+FAIL: c-c++-common/gomp/nesting-1.c  -std=c++98 (internal compiler error)+}
    {+FAIL:+} c-c++-common/gomp/nesting-1.c  -std=c++98 (test for excess errors)


Grüße,
 Thomas
Bernd Schmidt June 1, 2015, 12:08 p.m. UTC | #2 
On 06/01/2015 12:10 PM, Tom de Vries wrote:
> On 29/05/15 18:23, Bernd Schmidt wrote:
>> When predicating the code for OpenACC, we should avoid the entry block
>> in an offloaded region, which contains setup code that should be run in
>> every thread. The following patch adds a new marker statement that is
>> used to identify this block. Currently, predication doesn't happen
>> anyway due to an oversight in the algorithm, but I'll be fixing that in
>> a followup patch.
>>
>> Committed on gomp-4_0-branch.
>>
>
> Hi Bernd,
>
> this causes an ICE in kernels-eternal.c.

I don't see this test on my branch checkout - is it something you are 
using locally?


Bernd
Thomas Schwinge June 3, 2015, 11:41 a.m. UTC | #3 
Hi!

On Mon, 01 Jun 2015 13:57:47 +0200, I wrote:
> On Mon, 1 Jun 2015 12:10:12 +0200, Tom de Vries <Tom_deVries@mentor.com> wrote:
> > On 29/05/15 18:23, Bernd Schmidt wrote:
> > > When predicating the code for OpenACC, we should avoid the entry block
> > > in an offloaded region, which contains setup code that should be run in
> > > every thread. The following patch adds a new marker statement that is
> > > used to identify this block. Currently, predication doesn't happen
> > > anyway due to an oversight in the algorithm, but I'll be fixing that in
> > > a followup patch.
> > >
> > > Committed on gomp-4_0-branch.
> 
> > this causes an ICE in kernels-eternal.c.
> 
> (Belatedly) confirmed:
> 
>     [-PASS:-]{+FAIL: c-c++-common/goacc/kernels-eternal.c (internal compiler error)+}
>     {+FAIL:+} c-c++-common/goacc/kernels-eternal.c (test for excess errors)
> 
>     [-PASS:-]{+FAIL: c-c++-common/goacc/kernels-eternal.c  -std=c++11 (internal compiler error)+}
>     {+FAIL:+} c-c++-common/goacc/kernels-eternal.c  -std=c++11 (test for excess errors)
>     [-PASS:-]{+FAIL: c-c++-common/goacc/kernels-eternal.c  -std=c++14 (internal compiler error)+}
>     {+FAIL:+} c-c++-common/goacc/kernels-eternal.c  -std=c++14 (test for excess errors)
>     [-PASS:-]{+FAIL: c-c++-common/goacc/kernels-eternal.c  -std=c++98 (internal compiler error)+}
>     {+FAIL:+} c-c++-common/goacc/kernels-eternal.c  -std=c++98 (test for excess errors)
> 
> > The GIMPLE_OMP_ENTRY_END is removed the first time (!ssa) we call 
> > expand_omp_target. The second time we call expand_omp_target (ssa), it's 
> > no longer there, and we call 'gimple_code (NULL)' here:
> > ...
> >    basic_block entry_succ_bb = single_succ (entry_bb);
> >    gsi = gsi_last_bb (entry_succ_bb);
> >    if (gimple_code (gsi_stmt (gsi)) == GIMPLE_OMP_ENTRY_END)
> >      gsi_remove (&gsi, true);
> > ...
> > 
> > This patch fixes that by not attempting to remove GIMPLE_OMP_ENTRY_END 
> > when in ssa.
> > 
> > Committed to gomp-4_0-branch.
> 
> I also observed -- and still do observe -- ICEs as follows.  Tom, as
> you've fixed (thanks!) this already for
> c-c++-common/goacc/kernels-eternal.c, would you mind fixing it for these,
> too:
> 
>     [-PASS:-]{+FAIL: c-c++-common/goacc-gomp/nesting-1.c (internal compiler error)+}
>     {+FAIL:+} c-c++-common/goacc-gomp/nesting-1.c (test for excess errors)
> 
>     [-PASS:-]{+FAIL: c-c++-common/gomp/nesting-1.c (internal compiler error)+}
>     {+FAIL:+} c-c++-common/gomp/nesting-1.c (test for excess errors)
> 
>     [-PASS:-]{+FAIL: c-c++-common/goacc-gomp/nesting-1.c  -std=c++11 (internal compiler error)+}
>     {+FAIL:+} c-c++-common/goacc-gomp/nesting-1.c  -std=c++11 (test for excess errors)
>     [-PASS:-]{+FAIL: c-c++-common/goacc-gomp/nesting-1.c  -std=c++14 (internal compiler error)+}
>     {+FAIL:+} c-c++-common/goacc-gomp/nesting-1.c  -std=c++14 (test for excess errors)
>     [-PASS:-]{+FAIL: c-c++-common/goacc-gomp/nesting-1.c  -std=c++98 (internal compiler error)+}
>     {+FAIL:+} c-c++-common/goacc-gomp/nesting-1.c  -std=c++98 (test for excess errors)
> 
>     [-PASS:-]{+FAIL: c-c++-common/gomp/nesting-1.c  -std=c++11 (internal compiler error)+}
>     {+FAIL:+} c-c++-common/gomp/nesting-1.c  -std=c++11 (test for excess errors)
>     [-PASS:-]{+FAIL: c-c++-common/gomp/nesting-1.c  -std=c++14 (internal compiler error)+}
>     {+FAIL:+} c-c++-common/gomp/nesting-1.c  -std=c++14 (test for excess errors)
>     [-PASS:-]{+FAIL: c-c++-common/gomp/nesting-1.c  -std=c++98 (internal compiler error)+}
>     {+FAIL:+} c-c++-common/gomp/nesting-1.c  -std=c++98 (test for excess errors)

Bernd fixed this as part of his gomp-4_0-branch commit r224073:
»omp-low.c [...] (expand_omp_target): Only look for entry-end statement
if offloaded«.


Grüße,
 Thomas
diff mbox

Patch

Fix GIMPLE_OMP_ENTRY_END handling in expand_omp_target for ssa

2015-06-01  Tom de Vries  <tom@codesourcery.com>

	* omp-low.c (expand_omp_target): Fix GIMPLE_OMP_ENTRY_END handling for
	ssa.

---
 gcc/omp-low.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index a74a2bea..2634051 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -9486,9 +9486,12 @@  expand_omp_target (struct omp_region *region)
     }
 
   basic_block entry_succ_bb = single_succ (entry_bb);
-  gsi = gsi_last_bb (entry_succ_bb);
-  if (gimple_code (gsi_stmt (gsi)) == GIMPLE_OMP_ENTRY_END)
-    gsi_remove (&gsi, true);
+  if (!gimple_in_ssa_p (cfun))
+    {
+      gsi = gsi_last_bb (entry_succ_bb);
+      if (gimple_code (gsi_stmt (gsi)) == GIMPLE_OMP_ENTRY_END)
+	gsi_remove (&gsi, true);
+    }
 
   if (offloaded
       && do_splitoff)
-- 
1.9.1