Skip to content

Conversation

@radelja
Copy link
Collaborator

@radelja radelja commented Aug 16, 2025

This pull request addresses #25 by cache aligning the base addresses of the source and target addresses, and #26 by adding 'patternIdx' to correctly determine the address offsets in the kernels.

  • Adds new cache_line_size parameter to the Spatter generator to align start_source and start_target
  • Adds new align_start_addresses parameter to enable or disable cache line alignment of start addresses
  • Updates Gather, Scatter, MultiGather, and MultiScatter kernels to generate the correct address offsets
  • Closes Target address not cache line aligned #25
  • Closes Incorrect address offsets #26

@radelja radelja requested a review from Copilot August 16, 2025 07:55

This comment was marked as outdated.

@radelja radelja marked this pull request as ready for review August 16, 2025 08:14
@radelja radelja requested review from jyoung3131 and plavin August 16, 2025 08:15
{ "verbose", "Sets the verbosity of the output", "0" },
{ "args", "Sets the arguments to describe Spatter pattern(s)", "" },
{ "datawidth", "Sets the width of the memory operation", "8" },
{ "cache_line_size", "Size of the cache line the prefetcher is attached to", "64" },
Copy link
Contributor

Choose a reason for hiding this comment

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

@radelja - we wanted to note here that if the user doesn't want aligned addresses they may need to set this value to 1.

Copy link
Collaborator Author

@radelja radelja Aug 18, 2025

Choose a reason for hiding this comment

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

I have added a new align_start_addresses argument instead of requiring cache_line_size to be set to 1 to not align the addresses.

@radelja radelja marked this pull request as draft August 18, 2025 17:29
@radelja radelja requested a review from Copilot August 19, 2025 01:00
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request addresses two issues related to memory address handling in the Spatter generator: cache aligning base addresses and fixing address offset calculations. The changes ensure proper cache line alignment for performance and correct address computation in memory access patterns.

  • Adds cache line alignment functionality to the base source and target addresses
  • Fixes address offset calculations in memory access kernels by incorporating patternIdx
  • Updates parameter handling to support 64-bit addresses and alignment configuration

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/generators/spatterGenerator.h Adds new cache line alignment parameters, alignment function declaration, and updates data types
src/generators/spatterGenerator.cc Implements cache line alignment logic and fixes address offset calculations in gather/scatter kernels

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@radelja radelja marked this pull request as ready for review August 19, 2025 01:06
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.

Incorrect address offsets Target address not cache line aligned

2 participants