[1/9] tests: cris: force inlining

From: Rabin Vincent <rabinv@axis.com>

The CRIS tests expect that functions marked inline are always inline.
With newer versions of GCC, building them results warnings like the
following and spurious failures when they are run.

In file included from tests/tcg/cris/check_moveq.c:5:0:
tests/tcg/cris/crisutils.h:66:20: warning: inlining failed in call to
'cris_tst_cc.constprop.0': call is unlikely and code size would grow [-Winline]
tests/tcg/cris/check_moveq.c:28:13: warning: called from here [-Winline]

Use the always_inline attribute when building them to fix this.

Signed-off-by: Rabin Vincent <rabinv@axis.com>
---
 tests/tcg/cris/sys.h | 2 ++
 1 file changed, 2 insertions(+)

On Mon, Sep 05, 2016 at 01:54:04PM +0200, Rabin Vincent wrote:
> From: Rabin Vincent <rabinv@axis.com>
> 
> The CRIS tests expect that functions marked inline are always inline.
> With newer versions of GCC, building them results warnings like the
> following and spurious failures when they are run.
> 
> In file included from tests/tcg/cris/check_moveq.c:5:0:
> tests/tcg/cris/crisutils.h:66:20: warning: inlining failed in call to
> 'cris_tst_cc.constprop.0': call is unlikely and code size would grow [-Winline]
> tests/tcg/cris/check_moveq.c:28:13: warning: called from here [-Winline]
> 
> Use the always_inline attribute when building them to fix this.

Hi Rabin,

Do you happen to have a public git tree/branch with these changes?

Best regards,
Edgar


> 
> Signed-off-by: Rabin Vincent <rabinv@axis.com>
> ---
>  tests/tcg/cris/sys.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tests/tcg/cris/sys.h b/tests/tcg/cris/sys.h
> index c5f88e1..b1bf4c5 100644
> --- a/tests/tcg/cris/sys.h
> +++ b/tests/tcg/cris/sys.h
> @@ -3,6 +3,8 @@
>  #define STRINGIFY(x) #x
>  #define TOSTRING(x) STRINGIFY(x)
>  
> +#define inline inline __attribute__((always_inline))
> +
>  #define CURRENT_LOCATION __FILE__ ":" TOSTRING(__LINE__)
>  
>  #define err()                         \
> -- 
> 2.1.4
>

On Tue, Sep 06, 2016 at 11:53:46PM +0200, Edgar E. Iglesias wrote:
> On Mon, Sep 05, 2016 at 01:54:04PM +0200, Rabin Vincent wrote:
> > From: Rabin Vincent <rabinv@axis.com>
> > 
> > The CRIS tests expect that functions marked inline are always inline.
> > With newer versions of GCC, building them results warnings like the
> > following and spurious failures when they are run.
> > 
> > In file included from tests/tcg/cris/check_moveq.c:5:0:
> > tests/tcg/cris/crisutils.h:66:20: warning: inlining failed in call to
> > 'cris_tst_cc.constprop.0': call is unlikely and code size would grow [-Winline]
> > tests/tcg/cris/check_moveq.c:28:13: warning: called from here [-Winline]
> > 
> > Use the always_inline attribute when building them to fix this.
> 
> Hi Rabin,
> 
> Do you happen to have a public git tree/branch with these changes?

I've dropped the "sync CC state" patch for now and pushed the rest up to
the cris branch of https://github.com/rabinv/qemu/.

On 5 September 2016 at 12:54, Rabin Vincent <rabin.vincent@axis.com> wrote:
> From: Rabin Vincent <rabinv@axis.com>
>
> The CRIS tests expect that functions marked inline are always inline.
> With newer versions of GCC, building them results warnings like the
> following and spurious failures when they are run.
>
> In file included from tests/tcg/cris/check_moveq.c:5:0:
> tests/tcg/cris/crisutils.h:66:20: warning: inlining failed in call to
> 'cris_tst_cc.constprop.0': call is unlikely and code size would grow [-Winline]
> tests/tcg/cris/check_moveq.c:28:13: warning: called from here [-Winline]
>
> Use the always_inline attribute when building them to fix this.

This test code is pretty horrific; it relies on a whole bunch
of stuff that the compiler doesn't actually guarantee you,
and using always-inline is just a bandaid over the real
problem (which is that it should really be using multi-insn
asm statements when it cares about order, and not assuming
it can put two asm statements next to each other and set
cflags in one and read them in another).

That said, you probably aren't interested in rewriting it, so:

> Signed-off-by: Rabin Vincent <rabinv@axis.com>
> ---
>  tests/tcg/cris/sys.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tests/tcg/cris/sys.h b/tests/tcg/cris/sys.h
> index c5f88e1..b1bf4c5 100644
> --- a/tests/tcg/cris/sys.h
> +++ b/tests/tcg/cris/sys.h
> @@ -3,6 +3,8 @@
>  #define STRINGIFY(x) #x
>  #define TOSTRING(x) STRINGIFY(x)
>
> +#define inline inline __attribute__((always_inline))
> +

I think redefining C keywords is generally a bad idea.
Can you instead define an "always_inline" macro and
use it where necessary?

thanks
-- PMM

On 09/08/2016 01:06 PM, Peter Maydell wrote:

>> +++ b/tests/tcg/cris/sys.h
>> @@ -3,6 +3,8 @@
>>  #define STRINGIFY(x) #x
>>  #define TOSTRING(x) STRINGIFY(x)
>>
>> +#define inline inline __attribute__((always_inline))
>> +
> 
> I think redefining C keywords is generally a bad idea.
> Can you instead define an "always_inline" macro and
> use it where necessary?

In fact, commit b11d029b is an instance where compilation failed weirdly
because we redefined inline. I agree that a separate macro name, added
on where desired, rather than redefining 'inline' itself, is desirable.