Remove dead code.

Message ID 499b9ae2-1365-a954-ed5e-35aede5d0def@suse.cz
State New
Headers
Series Remove dead code. |

Commit Message

Martin Liška Nov. 8, 2021, 8:45 a.m. UTC
  This fixes issue reported in the PR.

Ready to be installed?
Thanks,
Martin

	PR other/89259

liboffloadmic/ChangeLog:

	* runtime/offload_omp_host.cpp: Remove size < 0 for a size of
	size_t type.
---
  liboffloadmic/runtime/offload_omp_host.cpp | 5 -----
  1 file changed, 5 deletions(-)
  

Comments

Jakub Jelinek Nov. 8, 2021, 8:59 a.m. UTC | #1
On Mon, Nov 08, 2021 at 09:45:39AM +0100, Martin Liška wrote:
> This fixes issue reported in the PR.
> 
> Ready to be installed?

I'm not sure.  liboffloadmic is copied from upstream, so the right
thing if we want to do anything at all (if we don't remove it, nothing
bad happens, the condition is never true anyway, whether removed away
in the source or removed by the compiler) would be to let Intel fix it in
their source and update from that.
But I have no idea where it even lives upstream.

	Jakub
  
Jeff Law Nov. 8, 2021, 2:19 p.m. UTC | #2
On 11/8/2021 2:59 AM, Jakub Jelinek via Gcc-patches wrote:
> On Mon, Nov 08, 2021 at 09:45:39AM +0100, Martin Liška wrote:
>> This fixes issue reported in the PR.
>>
>> Ready to be installed?
> I'm not sure.  liboffloadmic is copied from upstream, so the right
> thing if we want to do anything at all (if we don't remove it, nothing
> bad happens, the condition is never true anyway, whether removed away
> in the source or removed by the compiler) would be to let Intel fix it in
> their source and update from that.
> But I have no idea where it even lives upstream.
I thought MIC as an architecture was dead, so it could well be the case 
that there isn't a viable upstream anymore for that code.

jeff
  
Martin Liška Nov. 12, 2021, 2:27 p.m. UTC | #3
On 11/8/21 15:19, Jeff Law wrote:
> 
> 
> On 11/8/2021 2:59 AM, Jakub Jelinek via Gcc-patches wrote:
>> On Mon, Nov 08, 2021 at 09:45:39AM +0100, Martin Liška wrote:
>>> This fixes issue reported in the PR.
>>>
>>> Ready to be installed?
>> I'm not sure.  liboffloadmic is copied from upstream, so the right
>> thing if we want to do anything at all (if we don't remove it, nothing
>> bad happens, the condition is never true anyway, whether removed away
>> in the source or removed by the compiler) would be to let Intel fix it in
>> their source and update from that.
>> But I have no idea where it even lives upstream.
> I thought MIC as an architecture was dead, so it could well be the case that there isn't a viable upstream anymore for that code.
> 
> jeff

@H.J. ?

Martin
  
H.J. Lu Nov. 12, 2021, 2:41 p.m. UTC | #4
On Fri, Nov 12, 2021 at 6:27 AM Martin Liška <mliska@suse.cz> wrote:
>
> On 11/8/21 15:19, Jeff Law wrote:
> >
> >
> > On 11/8/2021 2:59 AM, Jakub Jelinek via Gcc-patches wrote:
> >> On Mon, Nov 08, 2021 at 09:45:39AM +0100, Martin Liška wrote:
> >>> This fixes issue reported in the PR.
> >>>
> >>> Ready to be installed?
> >> I'm not sure.  liboffloadmic is copied from upstream, so the right
> >> thing if we want to do anything at all (if we don't remove it, nothing
> >> bad happens, the condition is never true anyway, whether removed away
> >> in the source or removed by the compiler) would be to let Intel fix it in
> >> their source and update from that.
> >> But I have no idea where it even lives upstream.
> > I thought MIC as an architecture was dead, so it could well be the case that there isn't a viable upstream anymore for that code.
> >
> > jeff
>
> @H.J. ?
>

We'd like to deprecate MIC offload in GCC 12.  We will remove all traces of
MIC offload in GCC 13.
  
Martin Liška Nov. 12, 2021, 2:44 p.m. UTC | #5
On 11/12/21 15:41, H.J. Lu wrote:
> On Fri, Nov 12, 2021 at 6:27 AM Martin Liška <mliska@suse.cz> wrote:
>>
>> On 11/8/21 15:19, Jeff Law wrote:
>>>
>>>
>>> On 11/8/2021 2:59 AM, Jakub Jelinek via Gcc-patches wrote:
>>>> On Mon, Nov 08, 2021 at 09:45:39AM +0100, Martin Liška wrote:
>>>>> This fixes issue reported in the PR.
>>>>>
>>>>> Ready to be installed?
>>>> I'm not sure.  liboffloadmic is copied from upstream, so the right
>>>> thing if we want to do anything at all (if we don't remove it, nothing
>>>> bad happens, the condition is never true anyway, whether removed away
>>>> in the source or removed by the compiler) would be to let Intel fix it in
>>>> their source and update from that.
>>>> But I have no idea where it even lives upstream.
>>> I thought MIC as an architecture was dead, so it could well be the case that there isn't a viable upstream anymore for that code.
>>>
>>> jeff
>>
>> @H.J. ?
>>
> 
> We'd like to deprecate MIC offload in GCC 12.  We will remove all traces of
> MIC offload in GCC 13.

I see. So do you want the patch to be installed or not?

Cheers,
Martin

> 
>
  
H.J. Lu Nov. 12, 2021, 3 p.m. UTC | #6
On Fri, Nov 12, 2021 at 6:44 AM Martin Liška <mliska@suse.cz> wrote:
>
> On 11/12/21 15:41, H.J. Lu wrote:
> > On Fri, Nov 12, 2021 at 6:27 AM Martin Liška <mliska@suse.cz> wrote:
> >>
> >> On 11/8/21 15:19, Jeff Law wrote:
> >>>
> >>>
> >>> On 11/8/2021 2:59 AM, Jakub Jelinek via Gcc-patches wrote:
> >>>> On Mon, Nov 08, 2021 at 09:45:39AM +0100, Martin Liška wrote:
> >>>>> This fixes issue reported in the PR.
> >>>>>
> >>>>> Ready to be installed?
> >>>> I'm not sure.  liboffloadmic is copied from upstream, so the right
> >>>> thing if we want to do anything at all (if we don't remove it, nothing
> >>>> bad happens, the condition is never true anyway, whether removed away
> >>>> in the source or removed by the compiler) would be to let Intel fix it in
> >>>> their source and update from that.
> >>>> But I have no idea where it even lives upstream.
> >>> I thought MIC as an architecture was dead, so it could well be the case that there isn't a viable upstream anymore for that code.
> >>>
> >>> jeff
> >>
> >> @H.J. ?
> >>
> >
> > We'd like to deprecate MIC offload in GCC 12.  We will remove all traces of
> > MIC offload in GCC 13.
>
> I see. So do you want the patch to be installed or not?
>

I prefer to leave it alone and close the PR with WONTFIX.
  
Richard Biener Nov. 12, 2021, 7:14 p.m. UTC | #7
On November 12, 2021 3:41:41 PM GMT+01:00, "H.J. Lu via Gcc-patches" <gcc-patches@gcc.gnu.org> wrote:
>On Fri, Nov 12, 2021 at 6:27 AM Martin Liška <mliska@suse.cz> wrote:
>>
>> On 11/8/21 15:19, Jeff Law wrote:
>> >
>> >
>> > On 11/8/2021 2:59 AM, Jakub Jelinek via Gcc-patches wrote:
>> >> On Mon, Nov 08, 2021 at 09:45:39AM +0100, Martin Liška wrote:
>> >>> This fixes issue reported in the PR.
>> >>>
>> >>> Ready to be installed?
>> >> I'm not sure.  liboffloadmic is copied from upstream, so the right
>> >> thing if we want to do anything at all (if we don't remove it, nothing
>> >> bad happens, the condition is never true anyway, whether removed away
>> >> in the source or removed by the compiler) would be to let Intel fix it in
>> >> their source and update from that.
>> >> But I have no idea where it even lives upstream.
>> > I thought MIC as an architecture was dead, so it could well be the case that there isn't a viable upstream anymore for that code.
>> >
>> > jeff
>>
>> @H.J. ?
>>
>
>We'd like to deprecate MIC offload in GCC 12.  We will remove all traces of
>MIC offload in GCC 13.

Can you document that in gcc-12/changes.html in the caveats section please?

Thanks, 
Richard. 

>
  
H.J. Lu Nov. 12, 2021, 7:18 p.m. UTC | #8
On Fri, Nov 12, 2021 at 11:15 AM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On November 12, 2021 3:41:41 PM GMT+01:00, "H.J. Lu via Gcc-patches" <gcc-patches@gcc.gnu.org> wrote:
> >On Fri, Nov 12, 2021 at 6:27 AM Martin Liška <mliska@suse.cz> wrote:
> >>
> >> On 11/8/21 15:19, Jeff Law wrote:
> >> >
> >> >
> >> > On 11/8/2021 2:59 AM, Jakub Jelinek via Gcc-patches wrote:
> >> >> On Mon, Nov 08, 2021 at 09:45:39AM +0100, Martin Liška wrote:
> >> >>> This fixes issue reported in the PR.
> >> >>>
> >> >>> Ready to be installed?
> >> >> I'm not sure.  liboffloadmic is copied from upstream, so the right
> >> >> thing if we want to do anything at all (if we don't remove it, nothing
> >> >> bad happens, the condition is never true anyway, whether removed away
> >> >> in the source or removed by the compiler) would be to let Intel fix it in
> >> >> their source and update from that.
> >> >> But I have no idea where it even lives upstream.
> >> > I thought MIC as an architecture was dead, so it could well be the case that there isn't a viable upstream anymore for that code.
> >> >
> >> > jeff
> >>
> >> @H.J. ?
> >>
> >
> >We'd like to deprecate MIC offload in GCC 12.  We will remove all traces of
> >MIC offload in GCC 13.
>
> Can you document that in gcc-12/changes.html in the caveats section please?
>

I will do that.

Can you review my last wwwdocs change:

https://gcc.gnu.org/pipermail/gcc-patches/2021-August/578344.html

Thanks.
  
Richard Biener Nov. 12, 2021, 7:52 p.m. UTC | #9
On November 12, 2021 8:18:59 PM GMT+01:00, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>On Fri, Nov 12, 2021 at 11:15 AM Richard Biener
><richard.guenther@gmail.com> wrote:
>>
>> On November 12, 2021 3:41:41 PM GMT+01:00, "H.J. Lu via Gcc-patches" <gcc-patches@gcc.gnu.org> wrote:
>> >On Fri, Nov 12, 2021 at 6:27 AM Martin Liška <mliska@suse.cz> wrote:
>> >>
>> >> On 11/8/21 15:19, Jeff Law wrote:
>> >> >
>> >> >
>> >> > On 11/8/2021 2:59 AM, Jakub Jelinek via Gcc-patches wrote:
>> >> >> On Mon, Nov 08, 2021 at 09:45:39AM +0100, Martin Liška wrote:
>> >> >>> This fixes issue reported in the PR.
>> >> >>>
>> >> >>> Ready to be installed?
>> >> >> I'm not sure.  liboffloadmic is copied from upstream, so the right
>> >> >> thing if we want to do anything at all (if we don't remove it, nothing
>> >> >> bad happens, the condition is never true anyway, whether removed away
>> >> >> in the source or removed by the compiler) would be to let Intel fix it in
>> >> >> their source and update from that.
>> >> >> But I have no idea where it even lives upstream.
>> >> > I thought MIC as an architecture was dead, so it could well be the case that there isn't a viable upstream anymore for that code.
>> >> >
>> >> > jeff
>> >>
>> >> @H.J. ?
>> >>
>> >
>> >We'd like to deprecate MIC offload in GCC 12.  We will remove all traces of
>> >MIC offload in GCC 13.
>>
>> Can you document that in gcc-12/changes.html in the caveats section please?
>>
>
>I will do that.
>
>Can you review my last wwwdocs change:
>
>https://gcc.gnu.org/pipermail/gcc-patches/2021-August/578344.html

That change is OK. 

Richard. 

>Thanks.
>
  

Patch

diff --git a/liboffloadmic/runtime/offload_omp_host.cpp b/liboffloadmic/runtime/offload_omp_host.cpp
index 0439fec313b..4d8c57e3385 100644
--- a/liboffloadmic/runtime/offload_omp_host.cpp
+++ b/liboffloadmic/runtime/offload_omp_host.cpp
@@ -688,11 +688,6 @@  int omp_target_associate_ptr(
          return 1;
      }
  
-    // An incorrect size is treated as failure
-    if (size < 0) {
-        return 1;
-    }
-
      // If OpenMP allows wrap-around for device numbers, enable next line
      //Engine& device = mic_engines[device_num % mic_engines_total];
      Engine& device = mic_engines[device_num];