Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v4.0.x: Make sure MPIR_Breakpoint() is compiled without CFLAGS. #8422

Merged
merged 1 commit into from
Feb 1, 2021

Conversation

awlauria
Copy link
Contributor

@awlauria awlauria commented Jan 27, 2021

In optimized builds, CFLAGS contains various optimizations such as -O3,
and is propogated by automake to all files. To work-around this,
isolate MPIR_Breakpoint() and other MPIR_* symbols into its own library
built with debugger specific CFLAGS.

To prevent CFLAGS from being polluted elsewhere in the make tree, build
this in its own tiny stand-alone makefile.

Fixes #7757

Signed-off-by: Austen Lauria awlauria@us.ibm.com

NOTE: This is not a cherry-pick from master because ORTE no longer exists on master.

@awlauria awlauria force-pushed the fix_mpir_breakpoint_v4.0.x branch 2 times, most recently from 1d391c7 to 77b018e Compare January 27, 2021 21:35
@open-mpi open-mpi deleted a comment from ibm-ompi Jan 27, 2021
@open-mpi open-mpi deleted a comment from ibm-ompi Jan 27, 2021
@open-mpi open-mpi deleted a comment from ibm-ompi Jan 27, 2021
@jjhursey
Copy link
Member

bot:ibm:retest

In optimized builds, CFLAGS contains various optimizations such as -O3,
and is propogated by automake to all files. To work-around this,
isolate MPIR_Breakpoint() and other MPIR_* symbols into its own library
built with debugger specific CFLAGS.

To prevent CFLAGS from being polluted elsewhere in the make tree, build
this in its own tiny stand-alone makefile.

Fixes open-mpi#7757

Signed-off-by: Austen Lauria <awlauria@us.ibm.com>
@awlauria awlauria changed the title Make sure MPIR_Breakpoint() is compiled without CFLAGS. v4.0.x: Make sure MPIR_Breakpoint() is compiled without CFLAGS. Jan 28, 2021
Copy link
Member

@gpaulsen gpaulsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a line-by-line review of this with Austen, and this looks fine.
It's a shame we have to go to such lengths to specify a different CFLAGS for a file, but creating a more generic system would be more work, and this is the only file that really NEEDs it.
Austen will post his build log showing that this indeed dropped the Opt level to -O0 for this particular file.
Austen will also PR this to v4.1.x, as no orte in master.

Copy link
Member

@jjhursey jjhursey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fighting through the build system to do this.
It is unfortunate that we have to manipulate the build system in this way, but there are worse sins I suppose :)

@awlauria
Copy link
Contributor Author

I agree that we should fix this more generally. But for the 4.x release series it's good. Going forward I might keep plugging away at removing the global CFLAGS in my free time and post that PR if the community wants it.

@awlauria
Copy link
Contributor Author

awlauria commented Jan 28, 2021

Here's a piece of the make V=1 output showing only this file is compiled without O3:

out.txt

@jjhursey jjhursey added this to the v4.0.6 milestone Jan 28, 2021
@gpaulsen
Copy link
Member

@jsquyres Please review.

@hppritcha hppritcha merged commit 6b7fe90 into open-mpi:v4.0.x Feb 1, 2021
@hppritcha hppritcha added the NEWS label Jun 3, 2021
@awlauria awlauria deleted the fix_mpir_breakpoint_v4.0.x branch March 17, 2022 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failure to stop at MPIR_Breakpoint with OpenMPI v3.1.x and v4.0.x
5 participants