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

GCC warns when compiling the JIT: too small to hold all values of regNumber #33541

Closed
Tracked by #93172 ...
am11 opened this issue Mar 13, 2020 · 3 comments · Fixed by #59202
Closed
Tracked by #93172 ...

GCC warns when compiling the JIT: too small to hold all values of regNumber #33541

am11 opened this issue Mar 13, 2020 · 3 comments · Fixed by #59202
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI help wanted [up-for-grabs] Good issue for external contributors JitUntriaged CLR JIT issues needing additional triage
Milestone

Comments

@am11
Copy link
Member

am11 commented Mar 13, 2020

e.g. around 78% of this build https://dev.azure.com/dnceng/public/_build/results?buildId=557918&view=logs&jobId=e5f41d17-fae4-5d3d-f998-568159b1c41c&j=e5f41d17-fae4-5d3d-f998-568159b1c41c&t=86eaea2a-2e8c-50dd-cc99-c57aff36e5ad, there are a lots of such (non-fatal) warnings:

2020-03-13T00:38:03.8562716Z [ 78%] Built target utilcode_dac
2020-03-13T00:38:04.4983399Z Scanning dependencies of target clrjit
2020-03-13T00:38:04.6247371Z [ 78%] Building CXX object src/jit/standalone/CMakeFiles/clrjit.dir/__/alloc.cpp.o
2020-03-13T00:38:05.3361944Z In file included from /__w/1/s/src/coreclr/src/jit/codegeninterface.h:27:0,
2020-03-13T00:38:05.3363711Z                  from /__w/1/s/src/coreclr/src/jit/compiler.h:52,
2020-03-13T00:38:05.3453068Z                  from /__w/1/s/src/coreclr/src/jit/jit.h:806,
2020-03-13T00:38:05.3454005Z                  from /__w/1/s/src/coreclr/src/jit/jitpch.h:19,
2020-03-13T00:38:05.3455061Z                  from /__w/1/s/src/coreclr/src/jit/alloc.cpp:5:
2020-03-13T00:38:05.3457223Z /__w/1/s/src/coreclr/src/jit/emit.h:522:51: warning: 'emitter::emitAddrMode::amBaseReg' is too small to hold all values of 'regNumber {aka enum _regNumber_enum}'
2020-03-13T00:38:05.3458916Z          regNumber       amBaseReg : REGNUM_BITS + 1;
2020-03-13T00:38:05.3459845Z                                                    ^
2020-03-13T00:38:05.3461568Z /__w/1/s/src/coreclr/src/jit/emit.h:523:51: warning: 'emitter::emitAddrMode::amIndxReg' is too small to hold all values of 'regNumber {aka enum _regNumber_enum}'
2020-03-13T00:38:05.3462801Z          regNumber       amIndxReg : REGNUM_BITS + 1;
2020-03-13T00:38:05.3463827Z                                                    ^
2020-03-13T00:38:05.3465436Z /__w/1/s/src/coreclr/src/jit/emit.h:524:35: warning: 'emitter::emitAddrMode::amScale' is too small to hold all values of 'enum emitter::opSize'
2020-03-13T00:38:05.3466617Z          emitter::opSize amScale : 2;
2020-03-13T00:38:05.3467320Z                                    ^
2020-03-13T00:38:05.3468033Z In file included from /__w/1/s/src/coreclr/src/jit/jit.h:707:0,
2020-03-13T00:38:05.3468843Z                  from /__w/1/s/src/coreclr/src/jit/jitpch.h:19,
2020-03-13T00:38:05.3469644Z                  from /__w/1/s/src/coreclr/src/jit/alloc.cpp:5:
2020-03-13T00:38:05.3471439Z /__w/1/s/src/coreclr/src/jit/target.h:557:36: warning: 'emitter::instrDesc::idAddrUnion::<unnamed struct>::_idReg3' is too small to hold all values of 'regNumber {aka enum _regNumber_enum}'
2020-03-13T00:38:05.3472852Z    #define REGNUM_BITS              6       // number of bits in a REG_*
2020-03-13T00:38:05.3473658Z                                     ^
2020-03-13T00:38:05.3475339Z /__w/1/s/src/coreclr/src/jit/emit.h:847:37: note: in expansion of macro 'REGNUM_BITS'
2020-03-13T00:38:05.3476435Z                  regNumber _idReg3 : REGNUM_BITS;
2020-03-13T00:38:05.3477214Z                                      ^~~~~~~~~~~
2020-03-13T00:38:05.3478962Z /__w/1/s/src/coreclr/src/jit/target.h:557:36: warning: 'emitter::instrDesc::idAddrUnion::<unnamed struct>::_idReg4' is too small to hold all values of 'regNumber {aka enum _regNumber_enum}'
2020-03-13T00:38:05.3480334Z    #define REGNUM_BITS              6       // number of bits in a REG_*
2020-03-13T00:38:05.3481124Z                                     ^
2020-03-13T00:38:05.3482349Z /__w/1/s/src/coreclr/src/jit/emit.h:848:37: note: in expansion of macro 'REGNUM_BITS'
2020-03-13T00:38:05.3483322Z                  regNumber _idReg4 : REGNUM_BITS;
2020-03-13T00:38:05.3484102Z                                      ^~~~~~~~~~~
2020-03-13T00:38:05.3484918Z In file included from /__w/1/s/src/coreclr/src/jit/codegeninterface.h:27:0,
2020-03-13T00:38:05.3486183Z                  from /__w/1/s/src/coreclr/src/jit/compiler.h:52,
2020-03-13T00:38:05.3487168Z                  from /__w/1/s/src/coreclr/src/jit/jit.h:806,
2020-03-13T00:38:05.3488054Z                  from /__w/1/s/src/coreclr/src/jit/jitpch.h:19,
2020-03-13T00:38:05.3495192Z                  from /__w/1/s/src/coreclr/src/jit/alloc.cpp:5:
2020-03-13T00:38:05.3498610Z /__w/1/s/src/coreclr/src/jit/emit.h:570:30: warning: 'emitter::instrDesc::_idIns' is too small to hold all values of 'enum instruction'
2020-03-13T00:38:05.3499774Z          instruction _idIns : 10;

Is this something actionable?

category:implementation
theme:build
skill-level:beginner
cost:small

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI untriaged New issue has not been triaged by the area owner labels Mar 13, 2020
@janvorli
Copy link
Member

It looks like the regNumber enum has two extra members that are never stored in the fields the compiler complains about, see:

enum _regNumber_enum : unsigned
{
#define REGDEF(name, rnum, mask, sname) REG_##name = rnum,
#define REGALIAS(alias, realname) REG_##alias = REG_##realname,
#include "register.h"

    REG_COUNT,
    REG_NA           = REG_COUNT,
    ACTUAL_REG_COUNT = REG_COUNT - 1 // everything but REG_STK (only real regs)
};

@am11
Copy link
Member Author

am11 commented Mar 13, 2020

I have found a related bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61414, which seems to be fixed last month in 8.4+ and 9.3+, by introducing (only?) a warning suppression.

The problem is that when the enum with fixed underlying type is used as a bit field in a struct (or class), and has any valid value (within limits) but different width than the underlying type, we get this unspressible warning (hardcoded in g++). Minimal repro that demonstrates the issue with GCC when it compiles that part of JIT code: https://godbolt.org/z/YZnRvq.

@jkotas jkotas added the help wanted [up-for-grabs] Good issue for external contributors label Mar 13, 2020
@BruceForstall BruceForstall removed the untriaged New issue has not been triaged by the area owner label Apr 4, 2020
@BruceForstall BruceForstall added this to the Future milestone Apr 4, 2020
@BruceForstall BruceForstall added the JitUntriaged CLR JIT issues needing additional triage label Oct 28, 2020
@am11
Copy link
Member Author

am11 commented Jan 4, 2021

Update: the false-positive bug (https://godbolt.org/z/YZnRvq) was fixed in gcc 9.3+, 8.4+ and 10.0+. The GCC CI leg in master branch was today upgraded from gcc 7.3.1 to 9.3.1.

The remaining bitfield warnings are real, but not critical. However, they are on the edge of being critical, it seems.

Actually both GCC and Clang complain about e.g.

emitter::opSize amScale : 2;

because the largest value is 6, which requires 3 bits
enum opSize : unsigned

GCC: warning: 'emitter::emitAddrMode::amScale' is too small to hold all values of 'enum emitter::opSize'
Clang with -Weverything: warning: bit-field 'amScale' is not wide enough to store all enumerators of 'opSize' [-Wbitfield-enum-conversion]

They are currently not critical because we are not assigning the values which exceed the range. Otherwise clang without -Weverything issues -Wbitfield-constant-conversion and gcc produces -Woverflow. On runtime, the program compiled by either assigns garbage or zero for values out of bitfield range; which would make it a critical bug (but I think we have some assertions in emit.h to prevent them from happening).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI help wanted [up-for-grabs] Good issue for external contributors JitUntriaged CLR JIT issues needing additional triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants