| Message ID | 56312F77.3010005@acm.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Oct 28, 2015 at 01:26:31PM -0700, Nathan Sidwell wrote: > looking at the next thing to merge, I stumbled on code in lower_omp_target > that appears at least confused. > > we have: > if (offloaded || data_region) > { A } > else if (data_region) > new_body = tgt_body; > if (offloaded || data_region) > { B } > > which can clearly be simplified to: > > if (offloaded || data_region) > { A; B; } > > If that's incorrect, is the first '|| data_region' wrong? > > nathan > 2015-10-28 Nathan Sidwell <nathan@codesourcery.com> > > * omp-low.c (lower_omp_target): Remove unreachable code & merge > ifs. Your patch is fine. The reason for the "|| data_region" addition has been OMP_CLAUSE_USE_DEVICE_PTR clause support for OpenMP 4.5, without which nothing will be added to new_body sequence, other than tgt_body, so it is functionally equivalent in that case to the now unreachable code. Ok for trunk, thanks. Jakub
2015-10-28 Nathan Sidwell <nathan@codesourcery.com> * omp-low.c (lower_omp_target): Remove unreachable code & merge ifs. Index: omp-low.c =================================================================== --- omp-low.c (revision 229499) +++ omp-low.c (working copy) @@ -15931,14 +15931,12 @@ lower_omp_target (gimple_stmt_iterator * } break; } + gimple_seq_add_seq (&new_body, tgt_body); + if (offloaded) new_body = maybe_catch_exception (new_body); - } - else if (data_region) - new_body = tgt_body; - if (offloaded || data_region) - { + gimple_seq_add_stmt (&new_body, gimple_build_omp_return (false)); gimple_omp_set_body (stmt, new_body); }
Jakub, looking at the next thing to merge, I stumbled on code in lower_omp_target that appears at least confused. we have: if (offloaded || data_region) { A } else if (data_region) new_body = tgt_body; if (offloaded || data_region) { B } which can clearly be simplified to: if (offloaded || data_region) { A; B; } If that's incorrect, is the first '|| data_region' wrong? nathan