Skip to content

Conversation

@alexrp
Copy link
Contributor

@alexrp alexrp commented Aug 28, 2016

No description provided.

@alexrp
Copy link
Contributor Author

alexrp commented Aug 28, 2016

@vargaz does the LLVM bit look ok?

@vargaz
Copy link
Contributor

vargaz commented Aug 28, 2016

Yes.

@alexrp
Copy link
Contributor Author

alexrp commented Aug 29, 2016

cc @kumpera @BrzVlad

Copy link
Member

Choose a reason for hiding this comment

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

Do we really need unique names for this variable ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Given the var is scoped, I wonder the same. It's a style issue, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably a bit overengineered. Will simplify before merging.

@kumpera
Copy link
Contributor

kumpera commented Aug 30, 2016

LGTM

Good catch on removing ia64 dead code and unifying gcc version numbering.

There's the style issue that Vlad pointed out, but that doesn't invalidate this PR itself.

Alex Rønne Petersen added 4 commits August 30, 2016 21:12
This is a workaround. It's needed for the same reason that we need the fix for
Clang's atomic intrinsics in mono/utils/atomic.h.
All ia64 compilers have intrinsics that we use. Also, this ia64 code was very
out of date (missing many functions we use now) so it wouldn't compile anyway.
@alexrp alexrp closed this Aug 30, 2016
@alexrp alexrp reopened this Aug 30, 2016
@alexrp alexrp merged commit 2db49df into mono:master Aug 30, 2016
@alexrp alexrp deleted the arm64-native-atomics branch November 29, 2017 00:10
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
[runtime] Fix C and LLVM atomics on ARM64

Commit migrated from mono/mono@2db49df
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.

5 participants