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

Fix warnings on s390x #6077

Merged
merged 9 commits into from
Jul 6, 2021
Merged

Fix warnings on s390x #6077

merged 9 commits into from
Jul 6, 2021

Conversation

fjeremic
Copy link
Contributor

@fjeremic fjeremic commented Jun 17, 2021

A series of commits which address warnings on Linux and z/OS s390 targets.

```
warning: format ‘%x’ expects argument of type ‘unsigned int’, but argument 3 has type ‘intptr_t {aka long int}’ [-Wformat=]
```
```
warning: converting to non-pointer type ‘long unsigned int’ from NULL [-Wconversion-null]
```
@fjeremic fjeremic force-pushed the s390-warnings branch 2 times, most recently from 56c947d to d56a233 Compare June 17, 2021 19:47
@fjeremic fjeremic force-pushed the s390-warnings branch 2 times, most recently from ebcbc5e to eca1462 Compare June 28, 2021 17:29
@fjeremic fjeremic force-pushed the s390-warnings branch 2 times, most recently from 5a9c7fa to 02c2857 Compare June 29, 2021 14:28
```
warning: comparison between ‘enum OMR::RealRegister::RegNum’ and ‘enum OMR::RealRegister::RegDep’ [-Wenum-compare]
```
See eclipse#1662 for some further details. This will not be easy to solve
becalse it is not straightforward to covert `TR::JitConfig` to a POD
so we can use `offsetof`. This will hopefully be solved via the options
framework overhaul, so fixing this warning properly is not worth the
effort at the moment. Instead we locally disable this warning around
the problematic area.
```
CCN5216 (W) An expression of type "void *" cannot be converted to type "void (*)(int, int, int *)".
```
@fjeremic fjeremic changed the title WIP: Enable warnings as errors in compiler component on s390x WIP: Fix warnings on s390x Jun 29, 2021
```
CCN6101 (W) A return value of type "bool" is expected.
```
```
CCN6404 (W) The parameter "OS" specified for "pragma linkage" is not valid. The pragma is ignored.
```

According to the documentation `#pragma linkage` is only valid for C
compilation units.
@fjeremic fjeremic force-pushed the s390-warnings branch 5 times, most recently from 04a6b1e to e1077c4 Compare June 29, 2021 21:54
@fjeremic
Copy link
Contributor Author

Jenkins build all

@fjeremic fjeremic changed the title WIP: Fix warnings on s390x Fix warnings on s390x Jun 29, 2021
@fjeremic
Copy link
Contributor Author

@dsouzai this one should be ready.

Copy link
Member

@dsouzai dsouzai left a comment

Choose a reason for hiding this comment

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

LGTM; however, could you add a more descriptive message to the last commit (e1077c4); (ignoring the copyright updates) it seems to do more than just dealing with the non-POD class type passed in through ellipses (ie, the qasm and the __yield/__nop definitions)

@dsouzai
Copy link
Member

dsouzai commented Jun 30, 2021

Noting all tests passed, so a commit message change does not require running all the tests again.

@fjeremic
Copy link
Contributor Author

fjeremic commented Jul 5, 2021

LGTM; however, could you add a more descriptive message to the last commit (e1077c4); (ignoring the copyright updates) it seems to do more than just dealing with the non-POD class type passed in through ellipses (ie, the qasm and the __yield/__nop definitions)

Good catch. This seems like a mistake by me squashing commits and wrestling with xlC trying to get things to work. Going to fix this up now. I will likely split this into two commits as the warning number is incorrect. There will be no functional change, but we can run the testing again as it's fairly quick.

```
CCN8924 (W) Cannot pass an argument of non-POD class type "TR::DataType" through ellipsis.
```
```
CCN5562 (W) No code is generated for the asm statement. The asm statement is ignored.
```

Assembly generation for C/C++ code is disabled by default unless the
`-qasm` flag is set. The above warning was telling us the inline
assembly code was not generating anything, which is definitely not
what we want. Enabling `-qasm` further exposes bugs with the syntax
for inline assembly on xlC which is why the changes in AtomicSupport.cpp
were needed.
@fjeremic
Copy link
Contributor Author

fjeremic commented Jul 5, 2021

I force pushed the changes. As seen the force-push contains no actual code change. I've merely split the last commit into two, to isolate the -qasm change and explain why it was needed.

Jenkins build all

@dsouzai dsouzai merged commit 42d7f62 into eclipse:master Jul 6, 2021
@fjeremic fjeremic deleted the s390-warnings branch July 6, 2021 15:01
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