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

Revert PRs 4515 and 4520 (restore second, dsecnd) #4696

Merged
merged 2 commits into from
May 15, 2024

Conversation

frjohnst
Copy link
Contributor

@frjohnst frjohnst commented May 15, 2024

It turns out that PRs 4515 and 4520 break xlf builds of tests under
lapack-netlib/TESTING which require SECOND and DSECND. IBM
has decided this is a bigger problem than the conflict
between lapack second_ and the xlf run time.

This reverts commit 9b24b31.

It turns out that PRs 4515 and 4520 break the tests under
lapack-netlib/TESTING which require SECOND and DSECND. IBM
has decided this is a bigger biger problem than the conflict
between lapack second_ and the xlf run time.
This reverts commit bdaa670.

It turns out that PRs 4515 and 4520 break the tests under
lapack-netlib/TESTING which require SECOND and DSECND. IBM
has decided this is a bigger biger problem than the conflict
between lapack second_ and the xlf run time.
@martin-frbg
Copy link
Collaborator

Strange - unless I'm missing something, your original PRs were conditional on the compiler being xlf, and the value returned by second or dsecnd is only ever used to provide performance data that does not impact the validity of test results.

@frjohnst
Copy link
Contributor Author

A number of tests under lapack-netlib/TESTING/LIN and EIG call SECOND and DSECND and won't build
without these routines. PRs 4515 and 4520 removed those routines from builds with xlf compiler, as was mentioned,
but IBM decided it is important to be able to build these tests, as downloaded from the OpenBLAS repository,
with the xlf compiler and wants to restore this capability. Thanks.

grep -i '= *second' /.f /.F
LIN/cchkrfp.f: S1 = SECOND( )
LIN/cchkrfp.f: S2 = SECOND( )
LIN/schkrfp.f: S1 = SECOND( )
LIN/schkrfp.f: S2 = SECOND( )
EIG/cchkee.F: S1 = SECOND( )
EIG/cchkee.F: S2 = SECOND( )
EIG/schkee.F: S1 = SECOND( )
EIG/schkee.F: S2 = SECOND( )
LIN/cchkaa.F: S1 = SECOND( )
LIN/cchkaa.F: S2 = SECOND( )
LIN/schkaa.F: S1 = SECOND( )
LIN/schkaa.F: S2 = SECOND( )

grep -i '= *dsecnd' /.f /.F
LIN/dchkab.f: S1 = DSECND( )
LIN/dchkab.f: S2 = DSECND( )
LIN/dchkrfp.f: S1 = DSECND( )
LIN/dchkrfp.f: S2 = DSECND( )
LIN/zchkab.f: S1 = DSECND( )
LIN/zchkab.f: S2 = DSECND( )
LIN/zchkrfp.f: S1 = DSECND( )
LIN/zchkrfp.f: S2 = DSECND( )
EIG/dchkee.F: S1 = DSECND( )
EIG/dchkee.F: S2 = DSECND( )
EIG/zchkee.F: S1 = DSECND( )
EIG/zchkee.F: S2 = DSECND( )
LIN/dchkaa.F: S1 = DSECND( )
LIN/dchkaa.F: S2 = DSECND( )
LIN/zchkaa.F: S1 = DSECND( )
LIN/zchkaa.F: S2 = DSECND( )

@frjohnst
Copy link
Contributor Author

frjohnst commented May 15, 2024

I should have said that PRs 4515 and 4520 break xlf builds of some tests under lapack-netlib/TESTING ,
not the function of the tests themselves. (Just tried to update that.)

@martin-frbg
Copy link
Collaborator

Right, but my understanding was that these routines were conflicting with equivalent implementations provided by xlf ? I have no problem with reverting the changes, I am only wondering if there might be a better solution for this issue (that ultimately affects Reference-LAPACK as well, from where OpenBLAS' lapack-netlib tree is copied)

@frjohnst
Copy link
Contributor Author

I understand your point, but we have had several discussions within IBM, and it
was decided that not being able to build the tests as downloaded from OpenBLAS
with xlf was a bigger problem than the conflict between lapack's second_ and the
xlf run time. We will investigate whether a better solution might be possible in the
future, but in the short term we need to be able to easily build these tests with xlf.

So in the short term we would simply like to revert the changes in PRs 4515 and 4520. Thanks.

@martin-frbg martin-frbg added this to the 0.3.28 milestone May 15, 2024
@martin-frbg martin-frbg merged commit 9a2a6a2 into OpenMathLib:develop May 15, 2024
70 of 74 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants