Simulator: prevent inlining in C++ mode

Message ID CADip9gbbSjB8s9-E8MzgM4cDHig=NpUnh+eSyOC8drYb1VOBPw@mail.gmail.com
State New, archived
Headers

Commit Message

Павел Крюков Jan. 11, 2019, 9:59 a.m. UTC
  Simulator: prevent inlining in C++ mode

sim-arange.c contains C code and cannot be built with C++ compiler.
Instead, it should be built separately by C compiler w/o inlining.

sim/common/Changelog:
2019-01-11  Pavel I. Kryukov  <kryukov@frtk.ru>

        * sim-inline.h: don't define HAVE_INLINE with __cplusplus
  

Comments

Simon Marchi Jan. 15, 2019, 11:07 p.m. UTC | #1
On 2019-01-11 04:59, Павел Крюков wrote:
> Simulator: prevent inlining in C++ mode
> 
> sim-arange.c contains C code and cannot be built with C++ compiler.
> Instead, it should be built separately by C compiler w/o inlining.
> 
> sim/common/Changelog:
> 2019-01-11  Pavel I. Kryukov  <kryukov@frtk.ru>
> 
>         * sim-inline.h: don't define HAVE_INLINE with __cplusplus
> 
> diff --git a/sim/common/sim-inline.h b/sim/common/sim-inline.h
> index b2a3fc3..e9fb5c7 100644
> --- a/sim/common/sim-inline.h
> +++ b/sim/common/sim-inline.h
> @@ -282,7 +282,7 @@
> 
> 
>  #ifndef HAVE_INLINE
> -#ifdef __GNUC__
> +#if defined(__GNUC__) && !defined(__cplusplus)
>  #define HAVE_INLINE
>  #endif
>  #endif

What kind of errors do you get?  Would it be possible to modify the code 
so it compiles both as C and C++?  If all this trouble was taken to make 
these functions inline-able, I asusme that it's because the speedup is 
significant.

Simon
  
Павел Крюков Jan. 15, 2019, 11:31 p.m. UTC | #2
I got an error with that code:

#define DEFINE_INLINE_P (! defined (SIM_ARANGE_C_INCLUDED))
#define DEFINE_NON_INLINE_P defined (SIM_ARANGE_C_INCLUDED)
#if DEFINE_NON_INLINE_P

as it contains undefined behavior (see
https://gcc.gnu.org/onlinedocs/cpp/Defined.html).
Probably, it's better to revise the macro completely, as the same rule
applies to C as well.

However, even it is fixed, the remaining code of sim-arange.c is a C code
and I do not
want to pass it to my C++ compiler, as it is usually accompanied by
C++-specific sanitizers
For instance, they forbid C-style cast, malloc/free etc. Additionally, it
would be quite hard
to keep sim-arange.c C++-compatible all the time.

Thanks,
--
Pavel



ср, 16 янв. 2019 г. в 02:07, Simon Marchi <simon.marchi@polymtl.ca>:

> On 2019-01-11 04:59, Павел Крюков wrote:
> > Simulator: prevent inlining in C++ mode
> >
> > sim-arange.c contains C code and cannot be built with C++ compiler.
> > Instead, it should be built separately by C compiler w/o inlining.
> >
> > sim/common/Changelog:
> > 2019-01-11  Pavel I. Kryukov  <kryukov@frtk.ru>
> >
> >         * sim-inline.h: don't define HAVE_INLINE with __cplusplus
> >
> > diff --git a/sim/common/sim-inline.h b/sim/common/sim-inline.h
> > index b2a3fc3..e9fb5c7 100644
> > --- a/sim/common/sim-inline.h
> > +++ b/sim/common/sim-inline.h
> > @@ -282,7 +282,7 @@
> >
> >
> >  #ifndef HAVE_INLINE
> > -#ifdef __GNUC__
> > +#if defined(__GNUC__) && !defined(__cplusplus)
> >  #define HAVE_INLINE
> >  #endif
> >  #endif
>
> What kind of errors do you get?  Would it be possible to modify the code
> so it compiles both as C and C++?  If all this trouble was taken to make
> these functions inline-able, I asusme that it's because the speedup is
> significant.
>
> Simon
>
  
Павел Крюков Jan. 16, 2019, 6:29 p.m. UTC | #3
Okay, there are no C++ compiler errors other than macro expansion,
so this patch may be ignored. I submitted a separate patch for macros
expansion (https://sourceware.org/ml/gdb-patches/2019-01/msg00367.html)

(Sorry, I still get lines wrapped in patches while sending them via Gmail.
Could you please tell if there is a workaround?)

Thanks,
--
Pavel

ср, 16 янв. 2019 г. в 02:31, Павел Крюков <kryukov@frtk.ru>:

>
> I got an error with that code:
>
> #define DEFINE_INLINE_P (! defined (SIM_ARANGE_C_INCLUDED))
> #define DEFINE_NON_INLINE_P defined (SIM_ARANGE_C_INCLUDED)
> #if DEFINE_NON_INLINE_P
>
> as it contains undefined behavior (see https://gcc.gnu.org/onlinedocs/cpp/Defined.html).
> Probably, it's better to revise the macro completely, as the same rule applies to C as well.
>
> However, even it is fixed, the remaining code of sim-arange.c is a C code and I do not
> want to pass it to my C++ compiler, as it is usually accompanied by C++-specific sanitizers
> For instance, they forbid C-style cast, malloc/free etc. Additionally, it would be quite hard
> to keep sim-arange.c C++-compatible all the time.
>
> Thanks,
> --
> Pavel
>
>
>
> ср, 16 янв. 2019 г. в 02:07, Simon Marchi <simon.marchi@polymtl.ca>:
>>
>> On 2019-01-11 04:59, Павел Крюков wrote:
>> > Simulator: prevent inlining in C++ mode
>> >
>> > sim-arange.c contains C code and cannot be built with C++ compiler.
>> > Instead, it should be built separately by C compiler w/o inlining.
>> >
>> > sim/common/Changelog:
>> > 2019-01-11  Pavel I. Kryukov  <kryukov@frtk.ru>
>> >
>> >         * sim-inline.h: don't define HAVE_INLINE with __cplusplus
>> >
>> > diff --git a/sim/common/sim-inline.h b/sim/common/sim-inline.h
>> > index b2a3fc3..e9fb5c7 100644
>> > --- a/sim/common/sim-inline.h
>> > +++ b/sim/common/sim-inline.h
>> > @@ -282,7 +282,7 @@
>> >
>> >
>> >  #ifndef HAVE_INLINE
>> > -#ifdef __GNUC__
>> > +#if defined(__GNUC__) && !defined(__cplusplus)
>> >  #define HAVE_INLINE
>> >  #endif
>> >  #endif
>>
>> What kind of errors do you get?  Would it be possible to modify the code
>> so it compiles both as C and C++?  If all this trouble was taken to make
>> these functions inline-able, I asusme that it's because the speedup is
>> significant.
>>
>> Simon
  
Simon Marchi Jan. 16, 2019, 7:51 p.m. UTC | #4
On 2019-01-16 13:29, Pavel I. Kryukov wrote:
> Okay, there are no C++ compiler errors other than macro expansion,
> so this patch may be ignored. I submitted a separate patch for macros
> expansion (https://sourceware.org/ml/gdb-patches/2019-01/msg00367.html)

I'll take a look.

> (Sorry, I still get lines wrapped in patches while sending them via 
> Gmail.
> Could you please tell if there is a workaround?)

The best way is to use "git-send-email".  You will need an SMTP server, 
whether it is gmail, your ISP's, etc.  This should help:

https://coderwall.com/p/dp-gka/setting-up-git-send-email-with-gmail

If you really can't get this to work, the next best thing is to generate 
a .patch file (git format-patch HEAD^) and send it as an attachment.  
This ensures that the formatting is preserved.

Thanks!

Simon
  
Alan Hayward Jan. 17, 2019, 9:53 a.m. UTC | #5
> On 16 Jan 2019, at 19:51, Simon Marchi <simon.marchi@polymtl.ca> wrote:

> 

> On 2019-01-16 13:29, Pavel I. Kryukov wrote:

>> Okay, there are no C++ compiler errors other than macro expansion,

>> so this patch may be ignored. I submitted a separate patch for macros

>> expansion (https://sourceware.org/ml/gdb-patches/2019-01/msg00367.html)

> 

> I'll take a look.

> 

>> (Sorry, I still get lines wrapped in patches while sending them via Gmail.

>> Could you please tell if there is a workaround?)

> 

> The best way is to use "git-send-email".  You will need an SMTP server, whether it is gmail, your ISP's, etc.  This should help:

> 

> https://coderwall.com/p/dp-gka/setting-up-git-send-email-with-gmail

> 

> If you really can't get this to work, the next best thing is to generate a .patch file (git format-patch HEAD^) and send it as an attachment.  This ensures that the formatting is preserved.

> 


As someone who suffered similar issues until I moved to git send-mail, I’m
wondering if it’s worth it adding to the "Submitting Patches” section in
the CONTRIBUTE file?  I can have a go at writing that if it makes sense.


Alan.
  
Simon Marchi Jan. 17, 2019, 12:31 p.m. UTC | #6
On 2019-01-17 04:53, Alan Hayward wrote:
> As someone who suffered similar issues until I moved to git send-mail, 
> I’m
> wondering if it’s worth it adding to the "Submitting Patches” section 
> in
> the CONTRIBUTE file?  I can have a go at writing that if it makes 
> sense.

That would be very good, indeed!

Thanks,

Simon
  
Pedro Alves Jan. 17, 2019, 1:19 p.m. UTC | #7
On 01/17/2019 09:53 AM, Alan Hayward wrote:
> 
>> On 16 Jan 2019, at 19:51, Simon Marchi <simon.marchi@polymtl.ca> wrote:
>>
>> On 2019-01-16 13:29, Pavel I. Kryukov wrote:
>>> Okay, there are no C++ compiler errors other than macro expansion,
>>> so this patch may be ignored. I submitted a separate patch for macros
>>> expansion (https://sourceware.org/ml/gdb-patches/2019-01/msg00367.html)
>>
>> I'll take a look.
>>
>>> (Sorry, I still get lines wrapped in patches while sending them via Gmail.
>>> Could you please tell if there is a workaround?)
>>
>> The best way is to use "git-send-email".  You will need an SMTP server, whether it is gmail, your ISP's, etc.  This should help:
>>
>> https://coderwall.com/p/dp-gka/setting-up-git-send-email-with-gmail
>>
>> If you really can't get this to work, the next best thing is to generate a .patch file (git format-patch HEAD^) and send it as an attachment.  This ensures that the formatting is preserved.
>>
> 
> As someone who suffered similar issues until I moved to git send-mail, I’m
> wondering if it’s worth it adding to the "Submitting Patches” section in
> the CONTRIBUTE file?  I can have a go at writing that if it makes sense.

That will be good, no denying that.  Thanks in advance.

Though I think it'd be even better to fix the longstanding CONTRIBUTE
problem.  I.e., replace the current gdb/CONTRIBUTE file's contents, with a
pointer to:

 https://sourceware.org/gdb/contribute/

and then have that point to some wiki page instead of at gdb/CONTRIBUTE.

Wiki pages are much easier to update to keep up with the processes, which
are not tied to a specific gdb version.  Also, web content can take advantage
of cross linking.

Note that:

 https://sourceware.org/gdb/wiki/ContributionChecklist#Submitting_patches

also has information about git send-email and how to set up various clients.

Thanks,
Pedro Alves
  

Patch

diff --git a/sim/common/sim-inline.h b/sim/common/sim-inline.h
index b2a3fc3..e9fb5c7 100644
--- a/sim/common/sim-inline.h
+++ b/sim/common/sim-inline.h
@@ -282,7 +282,7 @@ 


 #ifndef HAVE_INLINE
-#ifdef __GNUC__
+#if defined(__GNUC__) && !defined(__cplusplus)
 #define HAVE_INLINE
 #endif
 #endif