Skip to content

Added an ILAENV2STAGE, fixed ILAENV in [CZ]HEEVR_2STAGE #177

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

Merged
merged 3 commits into from
Sep 9, 2017

Conversation

zerothi
Copy link
Contributor

@zerothi zerothi commented Jul 20, 2017

The ILAENV routine forces the entire LAPACK library to be compiled with -frecursive or the equivalent for non-GNU compilers.

This merge fixes this by adding a separate ILAENV2STAGE routine to query information related to the 2stage solvers, only.
I.e. there are now 2 ILAENV routines for retrieving machine dependent variables.

Secondly, this merge fixes ILAENV for the 2stage routines [CZ]HEEVR for their block-size queries (DSY* was used). I know this should have went in another PR, but that would inflict with this PR.

Tests are also updated to enable these changes.

zerothi added 3 commits July 20, 2017 13:26
Added ilaenv2stage.f to enable the extraction of 2stage machine dependent
variables from the 2stage solver without having to rely on compiling the
entire library with the recursive flag.

Secondly, all calls to ILAENV( 17 <= 21, ...) have been fixed to
use the corresponding ILAENV2STAGE routine by translating ISPEC by -16
I.e.

  ILAENV( 17, ...) == ILAENV2STAGE( 1, ...)

Also fixed the documentation of ilaenv.f to specify the cases of ISPEC > 16.

Signed-off-by: Nick Papior <[email protected]>
This required an overloaded ilaenv2stage in the ilaenv.f in the test
directories to use the PARMS(1) for ISPEC == 17.

Signed-off-by: Nick Papior <[email protected]>
They called with DSY* and thus the block-sizes were not selected
for complex numbers.

Signed-off-by: Nick Papior <[email protected]>
@Reference-LAPACK Reference-LAPACK deleted a comment from codecov bot Jul 20, 2017
@zerothi
Copy link
Contributor Author

zerothi commented Aug 30, 2017

@langou gentle ping, could you please comment on whether this seems like a good idea or not?

My main motivation was the removal of the -frecursive flag.

If this has no interest I can make a new PR with the fixes for the ilaenv calls in the complex routines and close this one.

@langou
Copy link
Contributor

langou commented Sep 9, 2017

Hi Nick, I was waiting on some feedback for some stakeholders in your fix. I did not hear from them. So this is all good. Yes, thanks for the fix. I do agree that this is better like this. Thanks for the PR. Cheers, Julien.

@langou langou merged commit 7abe213 into Reference-LAPACK:master Sep 9, 2017
@zerothi
Copy link
Contributor Author

zerothi commented Sep 10, 2017

Great it could be of use. You are welcome. :)

@zerothi zerothi deleted the ilaenv2stage branch September 10, 2017 18:06
@pv
Copy link

pv commented Oct 8, 2017

This removes the -frecursive flags. In view of stuff like this http://icl.cs.utk.edu/lapack-forum/viewtopic.php?t=1930, are you sure this it won't result to LAPACK builds losing threadsafety?

@zerothi
Copy link
Contributor Author

zerothi commented Oct 9, 2017

Yes, however in http://icl.cs.utk.edu/lapack-forum/viewtopic.php?t=1930#p5457 Julien explains that only using the correct stack size solves the problem (i.e. without frecursive), so I guess it is not really related to that flag.

@zerothi
Copy link
Contributor Author

zerothi commented Oct 9, 2017

I see now that the recursive flag is for exactly that bug. It should be re-added.

@zerothi
Copy link
Contributor Author

zerothi commented Oct 12, 2017

@pv I have made a PR to fix this, thanks for noticing.

zerothi added a commit to zerothi/lapack that referenced this pull request Aug 12, 2018
There were some missing ILAENV -> ILAENV2STAGE conversions
missing.
This commit fixes all calls to ILAENV that uses the parameters
17, 18, 19, 20 (the old ILAENV).

I have also tested this for the issue reported in Reference-LAPACK#262.

Signed-off-by: Nick Papior <[email protected]>
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.

3 participants