Differential D19787

bhyve: Stop compiling with -O0.
ClosedPublic
Actions

Authored by markj on Apr 2 2019, 1:33 AM.

Details

Reviewers

rgrimes
emaste
anish
jhb

Group Reviewers

bhyve

Commits

rS346011: Stop compiling bhyve(8) with -O0.

Summary

This appears to be accidental and came in with r243327. The bhyve
executable shrinks by over 30% with this change.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 23459
Build 22472: arc lint + arc unit

Event Timeline

markj created this revision.Apr 2 2019, 1:33 AM

Herald added a reviewer: bhyve. · View Herald TranscriptApr 2 2019, 1:33 AM

Herald added subscribers: bcran, rgrimes. · View Herald Transcript

Harbormaster completed remote builds in B23459: Diff 55711.Apr 2 2019, 1:33 AM

Does this variable get used by default? I saw this here and just ignored it as I thought it would only be invoked if I asked for a DEBUG build from a level above. If we are always adding DEBUG_FLAGS to our compiles perhaps it should be called something else?

This revision is now accepted and ready to land.Apr 2 2019, 1:54 AM

I think we should probably keep this, since by default EDK2 DEBUG builds are built with optimization enabled, which obviously makes debugging via gdb etc. irritating/difficult.
I always have to add -O0 to let me single step through the firmware.

In D19787#424348, @bcran wrote:

I think we should probably keep this, since by default EDK2 DEBUG builds are built with optimization enabled, which obviously makes debugging via gdb etc. irritating/difficult.
I always have to add -O0 to let me single step through the firmware.

I'm not sure what you mean, and I don't understand the connection between EDK2 and the bhyve build. Are you requesting that we always set -O0, even in release builds? Or do you want to leave this line commented out?

In D19787#424341, @rgrimes wrote:

Does this variable get used by default?

Yes, see bsd.prog.mk:

.if defined(DEBUG_FLAGS)                                                                                                                                  
CFLAGS+=${DEBUG_FLAGS}                                                                                                                                    
CXXFLAGS+=${DEBUG_FLAGS}                                                                                                                                  
...
.endif

In D19787#424350, @markj wrote:

I'm not sure what you mean, and I don't understand the connection between EDK2 and the bhyve build. Are you requesting that we always set -O0, even in release builds? Or do you want to leave this line commented out?

Sorry, wrong context. I was thinking of the uefi-edk2-bhyve port, which builds EDK2. This is different.

emaste accepted this revision.Apr 2 2019, 2:28 PM

anish accepted this revision.Apr 2 2019, 7:16 PM

jhb accepted this revision.Apr 3 2019, 7:00 PM

Closed by commit rS346011: Stop compiling bhyve(8) with -O0. (authored by markj). · Explain WhyApr 7 2019, 2:30 PM

This revision was automatically updated to reflect the committed changes.

Herald added a subscriber: imp. · View Herald TranscriptApr 7 2019, 2:30 PM