Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

GCC compatibility fixes #7 #22810

Merged
merged 13 commits into from
Feb 26, 2019
Merged

GCC compatibility fixes #7 #22810

merged 13 commits into from
Feb 26, 2019

Conversation

franksinankaya
Copy link

No description provided.

@franksinankaya
Copy link
Author

@jkotas @janvorli @am11

@franksinankaya franksinankaya force-pushed the gnu_cleanup_7 branch 3 times, most recently from e56dcce to a0379c5 Compare February 23, 2019 05:03
am11
am11 reviewed Feb 23, 2019
View reviewed changes
src/ilasm/CMakeLists.txt Outdated Show resolved Hide resolved
@am11 am11 mentioned this pull request Feb 24, 2019
Merged
@franksinankaya
Copy link
Author

Any review feedback?

janvorli
janvorli reviewed Feb 25, 2019
View reviewed changes
Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

Besides the nit, I would like to know @noahfalk's preference on the SyntheticStorage stuff change (as I have asked him in the other PR), but it seems fine otherwise.

src/vm/amd64/jithelpers_fastwritebarriers.S Outdated Show resolved Hide resolved
@franksinankaya franksinankaya force-pushed the gnu_cleanup_7 branch 2 times, most recently from 1505e26 to 267db3d Compare February 25, 2019 17:34
@franksinankaya
Copy link
Author

@dotnet-bot test coreclr-ci

@franksinankaya
Remove extra class qualifier
1136ca7
@franksinankaya
Replace hex number representation in ASM files
bdd0408
@franksinankaya
Use local copy of visitor rather than function parameter
4392c24
@franksinankaya
Reorder STDAPI and DLLEXPORT
dae93b5
@franksinankaya
Suppress conversion
1c54f67 
Suppress warning during hash

add casting
@franksinankaya
Remove anonymous struct
c14d568 
src/vm/codeversion.h:112:16: warning: ‘struct NativeCodeVersion::<anonymous union>::SyntheticStorage’ invalid; an anonymous union can only have non-static data members [-fpermissive]
         struct SyntheticStorage
@franksinankaya
Remove class declaration
0101067 
Remove extra class declaration
@franksinankaya
Remove extern C
c119ccf
@franksinankaya
Add implicit paranthesis
a9cbbd8 
src/vm/amd64/virtualcallstubcpu.hpp:735:103: warning: suggest parentheses around ‘-’ in operand of ‘&’ [-Wparentheses]
     resolveInit.toMiss1                = offsetof(ResolveStub,miss)-(offsetof(ResolveStub,toMiss1)+1) & 0xFF;
                                                                                                       ^
src/vm/amd64/virtualcallstubcpu.hpp:741:103: warning: suggest parentheses around ‘-’ in operand of ‘&’ [-Wparentheses]
     resolveInit.toMiss2                = offsetof(ResolveStub,miss)-(offsetof(ResolveStub,toMiss2)+1) & 0xFF;

Add parenthesis

src/vm/dataimage.cpp:631:55: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses]
                  previousRvaInfo->rva == rvaInfo->rva && previousRvaInfo->size >= rvaInfo->size

Add parenthesis

src/debug/daccess/daccess.cpp:6871:29: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses]
     _ASSERTE(peFile == NULL && reflectionModule != NULL || peFile != NULL && reflectionModule == NULL);

Add parenthesis

src/vm/dataimage.cpp:631:57: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses]
                  (previousRvaInfo->rva == rvaInfo->rva) && (previousRvaInfo->size >= rvaInfo->size)
@franksinankaya
Initialize member 1
8ad922c 
src/ilasm/method.cpp:35:36: warning: operation on ‘((Method*)this)->Method::m_ulColumns[0]’ may be undefined [-Wsequence-point]
     m_ulColumns[0]=m_ulColumns[0]=0;
@franksinankaya
Remove unknown compiler option
a9038cf
@franksinankaya
Use thread_local for thread local storage on non MSVC targets
7285867
@franksinankaya
Abstract DLLEXPORT
9489493
@franksinankaya
Copy link
Author

Rebased to master.

@franksinankaya
Copy link
Author

Should this go in since all tests pass now?

Any other review feedback?

@franksinankaya
Copy link
Author

@noahfalk : see this question:

'I would like to know @noahfalk's preference on the SyntheticStorage stuff change (as I have asked him in the other PR), but it seems fine otherwise.'

@noahfalk
Copy link
Member

Thanks for pinging, I missed the original in the noise : ) I didn't see if there multiple options you were trying to get my preference between, but what you have here that makes the struct nameless seems fine to me.

cc @kouvel (the current owner of the code version manager)

@kouvel
Copy link
Member

kouvel commented Feb 26, 2019

Looks ok to me as well

@janvorli janvorli merged commit cbd672e into dotnet:master Feb 26, 2019
@franksinankaya franksinankaya deleted the gnu_cleanup_7 branch February 26, 2019 16:05
omajid added a commit to omajid/dotnet-coreclr that referenced this pull request Aug 12, 2019
@omajid
Replace hex number representation in ASM files
7a15456 
This commit fixes coreclr to build in newer versions of llvm (tested
with llvm 8 on Fedora 30).

These recent versions of llvm (as well as GCC) do not accept values like
"20h" as valid integer literals:

    src/debug/ee/amd64/dbghelpers.S:32:21: error: unknown token in expression
	    add rsp, 20h
			^

This was reported as a bug to llvm upstream and they explicitly rejected
supporting these literals: https://reviews.llvm.org/D59810

This is partial backport of cbd672e
(PR dotnet#22810), which was about adding compatiblity with GCC 5.
omajid added a commit to omajid/dotnet-coreclr that referenced this pull request Aug 12, 2019
@omajid
Replace hex number representation in ASM files
b374b2c 
This commit fixes coreclr to build in newer versions of llvm (tested
with llvm 8 on Fedora 30).

These recent versions of llvm (as well as GCC) do not accept values like
"20h" as valid integer literals:

    src/debug/ee/amd64/dbghelpers.S:32:21: error: unknown token in expression
            add rsp, 20h
                        ^

This was reported as a bug to llvm upstream and they explicitly rejected
supporting these literals: https://reviews.llvm.org/D59810

This is partial backport of cbd672e
(PR dotnet#22810), with some modifications to compile, which was about adding
compatiblity with GCC 5.
omajid added a commit to omajid/dotnet-coreclr that referenced this pull request Aug 12, 2019
@omajid
Replace hex number representation in ASM files
3366bd7 
This commit fixes coreclr to build in newer versions of llvm (tested
with llvm 8 on Fedora 30).

These recent versions of llvm (as well as GCC) do not accept values like
"20h" as valid integer literals:

    src/debug/ee/amd64/dbghelpers.S:32:21: error: unknown token in expression
            add rsp, 20h
                        ^

This was reported as a bug to llvm upstream and they explicitly rejected
supporting these literals: https://reviews.llvm.org/D59810

This is partial backport of cbd672e
(PR dotnet#22810), with some modifications to compile, which was about adding
compatibility with GCC 5.
@omajid omajid mentioned this pull request Aug 12, 2019
Merged
omajid added a commit to omajid/dotnet-coreclr that referenced this pull request Aug 13, 2019
@omajid
Replace hex number representation in ASM files
221e5db 
This commit fixes coreclr to build in newer versions of llvm (tested
with llvm 8 on Fedora 30).

These recent versions of llvm (as well as GCC) do not accept values like
"20h" as valid integer literals:

    src/debug/ee/amd64/dbghelpers.S:32:21: error: unknown token in expression
            add rsp, 20h
                        ^

This was reported as a bug to llvm upstream and they explicitly rejected
supporting these literals: https://reviews.llvm.org/D59810

This is partial backport of cbd672e
(PR dotnet#22810), with some modifications to compile, which was about adding
compatibility with GCC 5.
jkotas pushed a commit that referenced this pull request Sep 12, 2019
@omajid @jkotas
Replace hex number representation in ASM files (#26136)
0fd2dc6 
This commit fixes coreclr to build in newer versions of llvm (tested
with llvm 8 on Fedora 30).

These recent versions of llvm (as well as GCC) do not accept values like
"20h" as valid integer literals:

    src/debug/ee/amd64/dbghelpers.S:32:21: error: unknown token in expression
            add rsp, 20h
                        ^

This was reported as a bug to llvm upstream and they explicitly rejected
supporting these literals: https://reviews.llvm.org/D59810

This is partial backport of cbd672e
(PR #22810), with some modifications to compile, which was about adding
compatibility with GCC 5.
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
@franksinankaya @janvorli
GCC compatibility fixes dotnet/coreclr#7 (dotnet/coreclr#22810)
169f5a3 
* Use thread_local for thread local storage on non MSVC targets

* Use local copy of visitor rather than function parameter

* Remove extra class qualifier

* Replace hex number representation in ASM files

* Reorder STDAPI and DLLEXPORT

* Suppress conversion

Suppress warning during hash

add casting

* Remove anonymous struct

src/vm/codeversion.h:112:16: warning: ‘struct NativeCodeVersion::<anonymous union>::SyntheticStorage’ invalid; an anonymous union can only have non-static data members [-fpermissive]
         struct SyntheticStorage

* Remove class declaration

Remove extra class declaration

* Remove extern C

* Add implicit paranthesis

src/vm/amd64/virtualcallstubcpu.hpp:735:103: warning: suggest parentheses around ‘-’ in operand of ‘&’ [-Wparentheses]
     resolveInit.toMiss1                = offsetof(ResolveStub,miss)-(offsetof(ResolveStub,toMiss1)+1) & 0xFF;
                                                                                                       ^
src/vm/amd64/virtualcallstubcpu.hpp:741:103: warning: suggest parentheses around ‘-’ in operand of ‘&’ [-Wparentheses]
     resolveInit.toMiss2                = offsetof(ResolveStub,miss)-(offsetof(ResolveStub,toMiss2)+1) & 0xFF;

Add parenthesis

src/vm/dataimage.cpp:631:55: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses]
                  previousRvaInfo->rva == rvaInfo->rva && previousRvaInfo->size >= rvaInfo->size

Add parenthesis

src/debug/daccess/daccess.cpp:6871:29: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses]
     _ASSERTE(peFile == NULL && reflectionModule != NULL || peFile != NULL && reflectionModule == NULL);

Add parenthesis

src/vm/dataimage.cpp:631:57: warning: suggest parentheses around ‘&&’ within ‘||’ [-Wparentheses]
                  (previousRvaInfo->rva == rvaInfo->rva) && (previousRvaInfo->size >= rvaInfo->size)

* Initialize member 1

src/ilasm/method.cpp:35:36: warning: operation on ‘((Method*)this)->Method::m_ulColumns[0]’ may be undefined [-Wsequence-point]
     m_ulColumns[0]=m_ulColumns[0]=0;

* Remove unknown compiler option

* Abstract DLLEXPORT


Commit migrated from dotnet/coreclr@cbd672e
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@am11 am11 am11 left review comments

@janvorli janvorli janvorli left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@franksinankaya @noahfalk @kouvel @am11 @janvorli