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

New function: memFile #1840

Merged
merged 1 commit into from
Jun 4, 2021
Merged

New function: memFile #1840

merged 1 commit into from
Jun 4, 2021

Conversation

DigitalBrains1
Copy link
Member

@DigitalBrains1 DigitalBrains1 commented Jun 3, 2021

A helper function for producing files with memory contents for such
modules as Clash.Explicit.BlockRam.File, etcetera.

I know you agreed to my initial proposal of

showsBitPackFile
  :: BitPack a
  => a
  -> ShowS

-- used as
bla file es =
  writeFile file (foldr showsBitPackFile "" es)

but I changed my mind. The point of ShowS is that you can combine it with other datatypes that have a Show instance. But the memory file only contains words of a single length n. If people really want to put multiple datatypes into one memory, they can just pack it into BitVector n. So the function doesn't need to be combined in ways as ShowS, and you would in practice always invoke it as foldr showsBitPackFile "", so why not incorporate that directly so people don't have to write it.

Still TODO:

  • Write a changelog entry (see changelog/README.md)
  • Check copyright notices are up to date in edited files

clash-prelude/src/Clash/Explicit/BlockRam/File.hs Outdated Show resolved Hide resolved
Comment on lines +277 to +279
Just (Bit 0 0) -> go n (val .&. (mask `xor` fullMask)) ('\n' : s)
Just (Bit 0 1) -> go n (val .|. mask) ('\n' : s)
Copy link
Contributor

Choose a reason for hiding this comment

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

It's unlikely that low and high will change, but maybe it's better to do

case care of
  Just b
    | b == low -> ...
    | b == high -> ...

Copy link
Member Author

Choose a reason for hiding this comment

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

That'll error on don't care bits!

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't care about those 😉

But more seriously, I thought they had a non-zero mask anyway? Or is that only undefined bits?

Copy link
Member Author

Choose a reason for hiding this comment

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

s/bits/bit/ :-)

Copy link
Member Author

@DigitalBrains1 DigitalBrains1 Jun 3, 2021

Choose a reason for hiding this comment

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

I never saw your comment before, odd. I was s/-ing my own "don't care bits" comment. The problem is that Eq Bit correctly stumbles over undefined bits. After all, you want lsb $(bLit ".") == 0 to throw an error instead of taking a wild stab at a boolean result.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes, I see now. I guess you would need to use something like Clash.Class.BitPack.isLike to compare in a way that's friendly to don't care values

Copy link
Member Author

Choose a reason for hiding this comment

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

The semantics of that here would mean that you want don't care bits to be mapped to don't care bits. And that is precisely the semantics of the current code. Just $ lsb $(bLit ".") is handled the same as Nothing by the _ pattern match.

Copy link
Member Author

Choose a reason for hiding this comment

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

User code should probably not peek under the hood of Bit, but we can do that, it's the same body of code as where it is defined.

@alex-mckenna
Copy link
Contributor

Also, you've ticked the changelog box but haven't pushed a changelog. Have you forgotten a git add?

@DigitalBrains1
Copy link
Member Author

Also, you've ticked the changelog box but haven't pushed a changelog. Have you forgotten a git add?

Again? Man....

@DigitalBrains1 DigitalBrains1 force-pushed the memfile branch 2 times, most recently from 6dbaa02 to d05a426 Compare June 3, 2021 12:52
A helper function for producing files with memory contents for such
modules as Clash.Explicit.BlockRam.File, etcetera.
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