Skip to content

[SPEC2017] Use correct input/output file paths for different benchmark types #247

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

SixWeining
Copy link
Contributor

These programs did not take into account the relationship between the benchmark type(rate vs. speed, test vs. train) and the input/output filenames.

  • 503.bwaves_r
  • 521.wrf_r
  • 554.roms_r
  • 603.bwaves_s
  • 621.wrf_s
  • 654.roms_s

…k types

These programs did not take into account the relationship between the
benchmark type(rate vs. speed, test vs. train) and the input/output filenames.
- 503.bwaves_r
- 521.wrf_r
- 554.roms_r
- 603.bwaves_s
- 621.wrf_s
- 654.roms_s
@SixWeining SixWeining requested a review from lukel97 May 17, 2025 07:55
@SixWeining
Copy link
Contributor Author

I just find that the fix for 603 is redundant with #200.

@SixWeining
Copy link
Contributor Author

cc @nmosier

@SixWeining SixWeining requested a review from tarunprabhu June 3, 2025 03:46
@tarunprabhu
Copy link
Contributor

Since this contains fixes similar to those for 604 for other tests as well, I am ok with this superseding the other.

This looks fine to me, but I am not familiar with the SPEC benchmarks, so it may be better to wait for someone else to approve.

@tarunprabhu
Copy link
Contributor

I just noticed that this has been up for 2 weeks already. If it's ok with you, I'd like to leave it up for a bit longer to see if someone more knowledgeable about this can take a look. Otherwise, I'll approve it. Is that ok?

@SixWeining
Copy link
Contributor Author

I just noticed that this has been up for 2 weeks already. If it's ok with you, I'd like to leave it up for a bit longer to see if someone more knowledgeable about this can take a look. Otherwise, I'll approve it. Is that ok?

Thanks for your comments. It seems that relatively few people are starting to run spec's Fortran benchmarks via the llvm-test-suite approach. People may be testing using the runcpu approach.

I'm OK to wait more time.

@DavidSpickett DavidSpickett requested a review from luporl June 4, 2025 08:59
@DavidSpickett
Copy link
Contributor

Linaro runs/has run spec Fortran benchmarks, @luporl might be able to review.

@luporl
Copy link
Contributor

luporl commented Jun 4, 2025

I run SPEC with runcpu, I haven't tried with llvm-test-suite. Maybe @antmox or @ceseo have experience with it.

@luporl luporl requested review from antmox and ceseo June 4, 2025 12:24
Copy link

@ceseo ceseo left a comment

Choose a reason for hiding this comment

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

This LGTM, but since I haven't touched this in months, I'd like someone else to take a second look.

@kiranchandramohan could you please take a look?

@kiranchandramohan
Copy link
Contributor

@kiranchandramohan could you please take a look?

I have not used the spec2017 setup from llvm-testsuite. So I cannot immediately review this.

Copy link
Contributor

@tarunprabhu tarunprabhu left a comment

Choose a reason for hiding this comment

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

Approving this now, but maybe wait until next week before merging to give others a chance to take a look as well.

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.

6 participants