If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

REALLY_INLINE does not seem to be supported by Sun compiler

ASSIGNED
Assigned to

Status

Tamarin
Virtual Machine
ASSIGNED
8 years ago
8 years ago

People

(Reporter: Brent Baker, Assigned: Leon Sha)

Tracking

unspecified
Future
Sun
Solaris
Bug Flags:
flashplayer-qrb +
flashplayer-triage +

Details

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
Refactoring the MMgc:WriteBarrier for better inlining (bug 488751) has caused a lot of warnings when compiling for solaris as the compiler does not support the "always_inline"

The code in unix-platform.h is this:
#if defined __SUNPRO_C || defined __SUNPRO_CC
  #define REALLY_INLINE inline __attribute__((always_inline)) __hidden
#endif

so somebody thought about this and presumably tested it, it defaults to just 'inline' if the platform file doesn't specify it.

This is a sample of the warnings that we are now getting when compiling on solaris: 

MMgc/WriteBarrier.h", line 251: Warning: attribute always_inline is unsupported and will be skipped..
coreToplevel.h", line 397:     Where: While specializing "MMgc::WriteBarrierRC<avmplus::DoubleVectorClass*>".
core/Toplevel.h", line 397:     Where: Specialized in non-template code.
MMgc/WriteBarrier.h", line 253: Warning: attribute always_inline is unsupported and will be skipped..
Flags: flashplayer-triage+
Flags: flashplayer-qrb+
(Reporter)

Comment 1

8 years ago
We are compiling using CC:
CC: Sun C++ 5.9 SunOS_sparc Patch 124863-10 2009/02/03

Comment 2

8 years ago
There's a fair amount of code in Tamarin that really really really wants to be inlined (and is impractical to macro-ize) ... and more we'd like to add in the future.

Do we know if the Sun compiler is in fact failing to inline? (ie is the warning actually indicating a failure to inline)
(Assignee)

Comment 3

8 years ago
Created attachment 386670 [details] [diff] [review]
patch to remove always_inline.

always_inline attribute only works for c compiler not c++ compiler. The warning is just to skip the attribute and does not indicating a failure to inline. You cn read the following document to get the information how sun compiler do the inline work.
http://developers.sun.com/sunstudio/documentation/ss12/mr/READMEs/c++_faq.html#RunPerf1

Also in the same file unix-platform.h, ifdef __GCC__ is not correct. __GNUC__ is the correct mico for gcc.
Assignee: lhansen → leon.sha
Attachment #386670 - Flags: review?(stejohns)

Comment 4

8 years ago
Comment on attachment 386670 [details] [diff] [review]
patch to remove always_inline.

let's capture the info about attribute((inline)) being C-only in a comment so it doesn't get re-added.
Attachment #386670 - Flags: review?(stejohns) → review+

Comment 5

8 years ago
perhaps the macro can detect C++ vs C and do the strongest inline mojo in both cases?  if the sunpro compiler has a standard #define indicating C++ mode then we're good to go.
(Reporter)

Comment 6

8 years ago
This warning is causing the build system to slow down because of a drastic increase in IO that must be transmitted between the buildslave and the master. Previously the stdio for a build was around 300K (x5 different builds) we are now pushing over 30MB of io per build. This has caused the the solaris build times to go from around 20 minutes up to 43 minutes.
(Assignee)

Comment 7

8 years ago
(In reply to comment #5)
> perhaps the macro can detect C++ vs C and do the strongest inline mojo in both
> cases?  if the sunpro compiler has a standard #define indicating C++ mode then
> we're good to go.

I have asked __attributes__ (always_inline) should also work on C++ complier. Seems there is a bug that this attributes can work with keyword inline. So currently we can just move this out and add some comments about this.
(Assignee)

Comment 8

8 years ago
__attribute__ ((always_inline)) will supported in the next major release of sunstudio. So we remove this attribute just for now and keep this bug opened. When we upgrade to the new compiler we can add this back.
http://hg.mozilla.org/tamarin-redux/rev/7cfc8a10c1f7

Updated

8 years ago
Target Milestone: --- → Future
You need to log in before you can comment on or make changes to this bug.