Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Development mailing list

Subject: Re: [OMPI devel] 1.4.4rc2 is up
From: Larry Baker (baker_at_[hidden])
Date: 2011-05-23 18:53:25


Jeff,

I get the following warnings from "make" using the Intel 2011.3.174
compilers on OpenMPI 1.4.3:

>

> btl_openib_endpoint.c(319): warning #188: enumerated type mixed with
> another type
> btl_openib_async.c(411): warning #188: enumerated type mixed with
> another type
> btl_openib_async.c(454): warning #188: enumerated type mixed with
> another type
> btl_openib_async.c(472): warning #188: enumerated type mixed with
> another type
> btl_openib_async.c(510): warning #188: enumerated type mixed with
> another type
> connect/btl_openib_connect_oob.c(289): warning #188: enumerated type
> mixed with another type
> connect/btl_openib_connect_xoob.c(462): warning #188: enumerated
> type mixed with another type
> connect/btl_openib_connect_xoob.c(563): warning #188: enumerated
> type mixed with another type

These are all warnings caused by mixing enums and ints as though they
were interchangeable. This concerns me, because, one can mistakenly
assume the only possible values of an enum data type are the range of
valid enum constants. That can make debugging such errors very
difficult. I do not know if you are aware of these.

I found the following:

1) ompi/mca/btl/openib/btl_openib/btl_openib_endpoint.c(319): warning
#188: enumerated type mixed with another type:

> ep->rem_info.rem_transport_type = (remote_proc_info-
> >pm_port_info).transport_type;

The left side is an enum mca_btl_openib_transport_type_t (from ompi/
mca/btl/openib/btl_openib.h):

> /**
> * Infiniband (IB) BTL component.
> */
>
> typedef enum {
> MCA_BTL_OPENIB_TRANSPORT_IB,
> MCA_BTL_OPENIB_TRANSPORT_IWARP,
> MCA_BTL_OPENIB_TRANSPORT_RDMAOE,
> MCA_BTL_OPENIB_TRANSPORT_UNKNOWN,
> MCA_BTL_OPENIB_TRANSPORT_SIZE
> } mca_btl_openib_transport_type_t;

, while the right side is a uint8_t (also from ompi/mca/btl/openib/
btl_openib.h):

> /**
> * Common information for all ports that is sent in the modex message
> */
> typedef struct mca_btl_openib_modex_message_t {
> /** The subnet ID of this port */
> uint64_t subnet_id;
> /** LID of this port */
> uint16_t lid;
> /** APM LID for this port */
> uint16_t apm_lid;
> /** The MTU used by this port */
> uint8_t mtu;
> /** vendor id define device type and tuning */
> uint32_t vendor_id;
> /** vendor part id define device type and tuning */
> uint32_t vendor_part_id;
> /** Transport type of remote port */
> uint8_t transport_type;
> /** Dummy field used to calculate the real length */
> uint8_t end;
> } mca_btl_openib_modex_message_t;

I would think mca_btl_openib_modex_message_t.transport_type should be
a compatible enum as well. At least there should be some code
somewhere with asserts or validity checks in, say, a switch block when
DEBUG=1, and a cast otherwise.

2) ompi/mca/btl/openib/btl_openib/btl_openib_async.c(411): warning
#188: enumerated type mixed with another type, and
ompi/mca/btl/openib/btl_openib_async.c(454): warning #188: enumerated
type mixed with another type:

> static ... ( ... enum ibv_qp_attr_mask *mask ... )
> {
<snip>
> *mask = IBV_QP_ALT_PATH|IBV_QP_PATH_MIG_STATE;

ompi/mca/btl/openib/btl_openib_async.c(472): warning #188: enumerated
type mixed with another type, and
ompi/mca/btl/openib/btl_openib_async.c(510): warning #188: enumerated
type mixed with another type:

> enum ibv_qp_attr_mask mask = 0;

These four warnings are all due to mixing the definition of bitfields
using a C enum type (from /usr/include/infiniband/verbs.h):

> enum ibv_qp_attr_mask {
> IBV_QP_STATE = 1 << 0,
> IBV_QP_CUR_STATE = 1 << 1,
> IBV_QP_EN_SQD_ASYNC_NOTIFY = 1 << 2,
> IBV_QP_ACCESS_FLAGS = 1 << 3,
> IBV_QP_PKEY_INDEX = 1 << 4,
> IBV_QP_PORT = 1 << 5,
> IBV_QP_QKEY = 1 << 6,
> IBV_QP_AV = 1 << 7,
> IBV_QP_PATH_MTU = 1 << 8,
> IBV_QP_TIMEOUT = 1 << 9,
> IBV_QP_RETRY_CNT = 1 << 10,
> IBV_QP_RNR_RETRY = 1 << 11,
> IBV_QP_RQ_PSN = 1 << 12,
> IBV_QP_MAX_QP_RD_ATOMIC = 1 << 13,
> IBV_QP_ALT_PATH = 1 << 14,
> IBV_QP_MIN_RNR_TIMER = 1 << 15,
> IBV_QP_SQ_PSN = 1 << 16,
> IBV_QP_MAX_DEST_RD_ATOMIC = 1 << 17,
> IBV_QP_PATH_MIG_STATE = 1 << 18,
> IBV_QP_CAP = 1 << 19,
> IBV_QP_DEST_QPN = 1 << 20
> };

The warning is really an error, since neither the right hand side
"IBV_QP_ALT_PATH|IBV_QP_PATH_MIG_STATE" nor "0" is one of the enum
ibv_qp_attr_mask values, and is therefore not a valid member of that
data type. I'm not really sure there is a better way, since C does
not guarantee the order of bitfields. Anyway, since C permits enums
to be used wherever ints can be used, the right hand side of

> *mask = IBV_QP_ALT_PATH|IBV_QP_PATH_MIG_STATE;

is equivalent to

> *mask = (int) IBV_QP_ALT_PATH | (int) IBV_QP_PATH_MIG_STATE;

, which results in an int. (And, of course, = 0 is an int.)

The simplest fix would be to cast the result into an enum
ibv_qp_attr_mask, with comments added that enum ibv_qp_attr_mask *mask
is really the union of all the bitfields in enum ibv_qp_attr_mask, and
that the value of *mask may not be a valid enum ibv_qp_attr_mask.

3) ompi/mca/btl/openib/btl_openib/connect/
btl_openib_connect_oob.c(289): warning #188: enumerated type mixed
with another type,

> enum ibv_mtu mtu = (openib_btl->device->mtu < endpoint-
> >rem_info.rem_mtu) ?
> openib_btl->device->mtu : endpoint->rem_info.rem_mtu;

ompi/mca/btl/openib/btl_openib/connect/btl_openib_connect_xoob.c(462):
warning #188: enumerated type mixed with another type, and

> attr.path_mtu = (openib_btl->device->mtu < endpoint-
> >rem_info.rem_mtu) ?
> openib_btl->device->mtu : rem_info->rem_mtu;

ompi/mca/btl/openib/btl_openib/connect/btl_openib_connect_xoob.c(563):
warning #188: enumerated type mixed with another type:

> attr.path_mtu = (openib_btl->device->mtu < endpoint-
> >rem_info.rem_mtu) ?
> openib_btl->device->mtu : rem_info->rem_mtu;

The left hand sides are encoded MTUs (enum ibv_mtu, from /usr/include/
infiniband/verbs.h):

> enum ibv_mtu {
> IBV_MTU_256 = 1,
> IBV_MTU_512 = 2,
> IBV_MTU_1024 = 3,
> IBV_MTU_2048 = 4,
> IBV_MTU_4096 = 5
> };

, while the openib_btl->device->mtu and rem_info->rem_mtu on the right
hand sides are uint32_t's (encoded?).

By the way, lines 563-564 in ompi/mca/btl/openib/btl_openib/connect/
btl_openib_connect_xoob.c look suspicious to me:

> ompi/mca/btl/openib/btl_openib/connect/
> btl_openib_connect_xoob.c(563): warning #188: enumerated type mixed
> with another type:
>
>> attr.path_mtu = (openib_btl->device->mtu < endpoint-
>> >rem_info.rem_mtu) ?
>> openib_btl->device->mtu : rem_info->rem_mtu;
>

The test on the right hand side of the conditional is endpoint-
>rem_info.rem_mtu, while the "false" expression is rem_info-
>rem_mtu. I suspect one of them is not correct.

Larry Baker
US Geological Survey
650-329-5608
baker_at_[hidden]

On 5 May 2011, at 7:15 AM, Jeff Squyres wrote:

> Fixed the ROMIO attribute problem properly this time -- it's in the
> usual place:
>
> http://www.open-mpi.org/software/ompi/v1.4/
>
> --
> Jeff Squyres
> jsquyres_at_[hidden]
> For corporate legal information go to:
> http://www.cisco.com/web/about/doing_business/legal/cri/
>
>
> _______________________________________________
> devel mailing list
> devel_at_[hidden]
> http://www.open-mpi.org/mailman/listinfo.cgi/devel