Hi Gerd,
Here's the response I got back from Simon:
there is an outstanding request with Intel, which would allow us to get rid of -strict-ansi. But until then, it seems to be a choice between (A) one line change every time we update verbs.h and (B) let in the code changes that break GCC build.
I'd vote for (A), since it isn't really that often that we pick new verbs.h.
So I suppose for now if the upstream doesn't pick up the enum change, we can make the change in our private verbs.h copy.
Thanks,
Zuoyu
-----Original Message-----
From: Gerd Rausch
Sent: Friday, May 24, 2019 12:26 PM
To: Zuoyu Tao <zuoyu.tao@xxxxxxxxxx>; Jason Gunthorpe <jgg@xxxxxxxxxxxx>
Cc: linux-rdma@xxxxxxxxxxxxxxx; Aron Silverton <aron.silverton@xxxxxxxxxx>; Sharon Liu <sharon.s.liu@xxxxxxxxxx>
Subject: Re: <infiniband/verbs.h> & ICC
Hi,
It's the "-strict-ansi" that makes it fail:
% icc -c -strict-ansi foo.c
foo.c(2): error: enumeration value is out of "int" range
enum { FOO = 1UL << 31 } foo;
Zuoyu, is it possible to _not_ use "-strict-ansi"?
Thanks,
Gerd
On 24/05/2019 11.59, Zuoyu Tao wrote:
> Here were the compiler flags used with ICC:
>
> -trigraphs -fno-omit-frame-pointer -fp-model source -fno-strict-aliasing -mIPOPT_clone_max_total_clones=0 -mP2OPT_hpo_enable_short_trip_vec=F -sox=profile -sox=inline -no-global-hoist -mP2OPT_tls_control=0 -wd191 -wd175 -wd188 -wd810 -we127 -we1345 -we1338 -wd279 -wd186 -wd1572 -wd589 -wd11505 -we592 -wd69 -we172 -Qoption,cpp,--treat_func_as_string_literal -mP2OPT_spill_parms=T -wd11505 -wd411 -wd273 -ww174 -we266 -ww279 -we589 -we810 -we1011 -ww1418 -strict-ansi -wd66 -wd76 -wd82 -wd94 -we102 -wd271 -wd424 -wd561 -wd662 -wd1511 -std=c99 -ww344 -we137 -fPIC -mP2OPT_tls_control=2
>
> -----Original Message-----
> From: Gerd Rausch
> Sent: Thursday, May 23, 2019 11:15 PM
> To: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
> Cc: linux-rdma@xxxxxxxxxxxxxxx; Aron Silverton
> <aron.silverton@xxxxxxxxxx>; Sharon Liu <sharon.s.liu@xxxxxxxxxx>;
> ZUOYU.TAO <zuoyu.tao@xxxxxxxxxx>
> Subject: Re: <infiniband/verbs.h> & ICC
>
> +Zuoyu,
>
> Hi Zuoyo,
>
> What compiler flags were you using while compiling the <verbs.h> file throwing the error about 'enumeration value is out of "int" range'?
>
>
> On 23/05/2019 18.30, Jason Gunthorpe wrote:
>> On Thu, May 23, 2019 at 03:57:29PM -0700, Gerd Rausch wrote:
>>>
>>> error: enumeration value is out of "int" range
>>> IBV_RX_HASH_INNER = (1UL << 31),
>>
>> I assume you are running with some higher warning flags and -Werror?
>> gcc will not emit this warning without -Wpedantic
>>
>
> Perhaps. I've added Zuoyu, who reported this issue to this e-mail thread.
>
>>
>>> Since "int" is signed, it can't hold the unsigned value of 1UL<<31
>>> on target platforms with sizeof(int) <= 4.
>>
>> Pedentically yes, but gcc and any compiler that can compile on linux
>> supports an extension where the underlying type of an enum constant
>> is automatically increased until it can hold the value of the constant.
>> In this case the constant is type promoted to long, IIRC.
>>
>
> Evidently ICC supports that as well:
> % icc --version
> icc (ICC) 17.0.5 20170817
> Copyright (C) 1985-2017 Intel Corporation. All rights reserved.
>
> % cat foo.c
> enum { FOO = 1UL << 31 } foo = FOO;
>
> % icc -c -g foo.c && gdb -ex 'ptype foo' -ex 'print sizeof foo' foo.o
> type = enum {FOO = -2147483648}
> $1 = 4
>
> % cat bar.c
> enum { FOO = 1UL << 31, BAR = -1 } bar = BAR; % icc -c -g bar.c && gdb
> -ex 'ptype bar' -ex 'print sizeof bar' bar.o type = enum {FOO =
> -2147483648, BAR = -1}
> $1 = 8
>
> I can't say that I'm thrilled with this behavior though, as it appears error-prone:
> As soon as an enum value goes out of range for an "int", the type silently changes, potentially rendering structures and functions silently incompatible.
> It's quite the pitfall (e.g. the foo.c vs bar.c case above).
>
>>
>> Can you clarify if icc is being run in some wonky mode that is
>> causing this warning? AFAIK icc will compile the linux kernel, and
>> the kernel makes extensive use of this extension. So I think the
>> compiler is not configured properly.
>>
>
> I've added Zuoyu to the distribution to shed some light on that.
>
>> IIRC I looked at this once for -Wpedantic support and decided it was
>> a lot of work as there are more cases than just this.
>>
>
> Not exactly shocking news ;-)
>
> Thanks for providing the information,
>
> Gerd
>
