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

Add __UNALIGNED_UINT64_READ/WRITE #768

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

Conversation

kjbracey
Copy link
Collaborator

Extend unaligned access support macros to cover 64-bit accesses.

@kjbracey
Copy link
Collaborator Author

Lack of this became apparent in ARMmbed/mbed-os#12086

#ifndef __UNALIGNED_UINT64_READ
#pragma language=save
#pragma language=extended
__IAR_FT uint64_t __iar_uint64_read(void const *ptr)
Copy link
Member

Choose a reason for hiding this comment

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

@TTornblom, please cross check changes to IAR compiler header files.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Please update version and date in file header section of the changed files.

@kjbracey
Copy link
Collaborator Author

Please update version and date in file header section of the changed files.

Could you clarify the exact requirement here? It's not quite clear to me whether this is using per-file semantic versioning, or what. Can't find docs, and history seems inconsistent.

@JonatanAntoni
Copy link
Member

Yes, history is extremely inconsistent. Ultimately what we need to assure is that someone who grabs only a single file from CMSIS still clearly knows from which version is was taken.

Hence I update at least the change date per file (see file header comments).

Additionally I increase the file version (on a per file basis) at least once per release on a semantic basis. For config files and user templates its even more important and the exact file version must be kept in sync with the pack description (ARM.CMSIS.pdsc).

You're absolutely right, we should think about putting a clear procedure to the docs. Where did you search for it?

@ghost
Copy link

ghost commented Dec 18, 2019

Hello @kjbracey-arm,
all the CMSIS header files have a 'date' and a 'version' tag.
In my opinion these tags should be updated whenever changes are made to the file.

On the other hand it is not clear to me if these tags are necessary. It looks like they do not correspond to the CMSIS version to which the file belongs.

Maybe these tags can be removed from the file header.

What is the opinion of @JonatanAntoni on this?

@JonatanAntoni
Copy link
Member

JonatanAntoni commented Dec 18, 2019

@GuentherMartin, I don't have a clear tendency.

On one hand maintaining these dates and versions manually is error prone. On the other hand people still tend to grab arbitrary versions of CMSIS files and start copying (redistributing) them around.

Ultimately some users come across problems in their version and raise issues here. Without a hint from which point in time the users version of a file was taken, it gets quite hard to recap the problems. In this case is helps to know at least the last modification date of each file. Perhaps that could be achieved somehow automatically?

@kjbracey
Copy link
Collaborator Author

kjbracey commented Dec 18, 2019

Perhaps that could be achieved somehow automatically?

I don't think there's much you can do on a live Git repo to automate. The committer controls file content, and the repo doesn't get to mess with it. Any magic would need to be in a submitter-end hook (like the Change-Id thing that Gerrit uses).

An export via git archive can substitute $Date$-type markers in files. But that's not sufficient for people just grabbing stuff off GitHub clones.

Changes to these core files seem pretty infrequent, so probably reasonable to get submitters to manually update both date and semantic version on each.

If changes were frequent, you'd probably want to just do one semantic version update at release time.

Shall I do both date + version now?

@ghost
Copy link

ghost commented Dec 18, 2019

If a file contains a date and a version tag then they should be updated if the file is changed.

A solution would be to use a 3 digit version like ..
If the file is changed 'patch' should be increased.
With a new CMSIS release 'CMSIS main' and 'CMSIS sub' are changed and 'patch' is reset to 0.

So everyone can clearly see to which CMSIS version a file belongs and if it is changed.

But I do not know how much effort this is or if this can be automated.

@kjbracey
Copy link
Collaborator Author

kjbracey commented Dec 18, 2019

That proposal drops any possibility of final semantic versioning though. You could maintain metadata for changes on develop without having excess version changes on release by using a "pre-release" marker.

Eg
1.0.0 last release version
1.0.1-1 patch change on develop (increment patch version, and add pre-release marker)
1.0.1-2 another patch change on develop (increment pre-release only, no need to bump patch)
1.1.0-1 feature change on develop (bump minor, can reset minor and pre-release)
1.1.0-2 another feature or patch change on develop (increment pre-release only, no need to bump patch or feature)
1.1.0 next release version (remove pre-release marker)

That procedure is endorsed by https://semver.org/, generates the correct sort according to the "comparison" rules there. (1.0.1-1 < 1.0.1-2 and 1.1.0-1 < 1.1.0), and avoids any excess version increment on releases.

@JonatanAntoni
Copy link
Member

@kjbracey-arm, on pack (and component) level we use semver as you described it. But I'd rather want to keep the "versioning" on file level simpler.

I think there is no value in updating all file version tags once per release only to reflect the component version in every file. In the beginning the file version reflected the component version to a certain extend. But we never maintained this kind of version information properly.

@kjbracey
Copy link
Collaborator Author

So just update the date on commit submission and leave it to you or other maintainer to do a file version change only on release? (According to maintainer's assessment of appropriate semver change or whatever?)

@JonatanAntoni
Copy link
Member

In this case I'd update the file version of all files by doing minor+1 as its a backward compatible feature enhancement. Set the file change date to the submission date. Should be fine for now.

@kjbracey
Copy link
Collaborator Author

  • Corrected error in IAR version
  • Added to Core-A too
  • Versions and date incremented

@JonatanAntoni
Copy link
Member

@kjbracey-arm, would you mind to squash your commits into a single one?

Extend unaligned access support macros to cover 64-bit accesses.
@JonatanAntoni
Copy link
Member

Can one of the admins verify this patch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants