El Wed, Oct 04, 2017 at 11:17:24AM -0700 Jakub Kicinski ha dit: > On Wed, 4 Oct 2017 10:42:42 -0700, Manoj Gupta wrote: > > Hi Jakub, > > > > I had discussed about supporting this code with some clang developers. > > However, the consensus was this code relies on a specific GCC optimizer > > behavior and Clang does not share the same behavior by design. > > Hm. I find surprising that constant propagation to inlined functions > is considered something GCC-specific rather than obvious/basic. > > > Note that even GCC can't compile this code at -O0. > > Yes, that makes me feel uncomfortable... Could you try this?
The kernel as a whole does not build with -O0, so I couldn't try that. I know Manoj (who isn't a kernel dev) isolated the compiletime_assert macros and encountered the same 'undefined reference' errors with gcc -O0 that we are seeing with clang. This is due to: "if you use it in an inlined function and pass an argument of the function as the argument to the built-in, GCC never returns 1 when you call the inline function with a string constant or compound literal (see Compound Literals) and does not return 1 when you pass a constant numeric value to the inline function unless you specify the -O option." https://gcc.gnu.org/onlinedocs/gcc-7.2.0/gcc/Other-Builtins.html > diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c > b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c > index f6f7c085f8e0..47251396fcae 100644 > --- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c > +++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c > @@ -469,10 +469,10 @@ int nfp_eth_set_configured(struct nfp_cpp *cpp, > unsigned int idx, bool configed) > return nfp_eth_config_commit_end(nsp); > } > > -/* Force inline, FIELD_* macroes require masks to be compilation-time known > */ > -static __always_inline int > +static int > nfp_eth_set_bit_config(struct nfp_nsp *nsp, unsigned int raw_idx, > - const u64 mask, unsigned int val, const u64 ctrl_bit) > + const u64 mask, const unsigned int shift, > + unsigned int val, const u64 ctrl_bit) > { > union eth_table_entry *entries = nfp_nsp_config_entries(nsp); > unsigned int idx = nfp_nsp_config_idx(nsp); > @@ -489,11 +489,11 @@ nfp_eth_set_bit_config(struct nfp_nsp *nsp, unsigned > int raw_idx, > > /* Check if we are already in requested state */ > reg = le64_to_cpu(entries[idx].raw[raw_idx]); > - if (val == FIELD_GET(mask, reg)) > + if (val == (reg & mask) >> shift) > return 0; > > reg &= ~mask; > - reg |= FIELD_PREP(mask, val); > + reg |= (val << shift) & mask; > entries[idx].raw[raw_idx] = cpu_to_le64(reg); > > entries[idx].control |= cpu_to_le64(ctrl_bit); > @@ -503,6 +503,13 @@ nfp_eth_set_bit_config(struct nfp_nsp *nsp, unsigned int > raw_idx, > return 0; > } > > +#define NFP_ETH_SET_BIT_CONFIG(nsp, raw_idx, mask, val, ctrl_bit) \ > + ({ \ > + __BF_FIELD_CHECK(mask, 0ULL, val, "NFP_ETH_SET_BIT_CONFIG: "); \ > + nfp_eth_set_bit_config(nsp, raw_idx, mask, __bf_shf(mask), \ > + val, ctrl_bit); \ > + }) > + > /** > * __nfp_eth_set_aneg() - set PHY autonegotiation control bit > * @nsp: NFP NSP handle returned from nfp_eth_config_start() > @@ -515,7 +522,7 @@ nfp_eth_set_bit_config(struct nfp_nsp *nsp, unsigned int > raw_idx, > */ > int __nfp_eth_set_aneg(struct nfp_nsp *nsp, enum nfp_eth_aneg mode) > { > - return nfp_eth_set_bit_config(nsp, NSP_ETH_RAW_STATE, > + return NFP_ETH_SET_BIT_CONFIG(nsp, NSP_ETH_RAW_STATE, > NSP_ETH_STATE_ANEG, mode, > NSP_ETH_CTRL_SET_ANEG); > } > @@ -544,7 +551,7 @@ int __nfp_eth_set_speed(struct nfp_nsp *nsp, unsigned int > speed) > return -EINVAL; > } > > - return nfp_eth_set_bit_config(nsp, NSP_ETH_RAW_STATE, > + return NFP_ETH_SET_BIT_CONFIG(nsp, NSP_ETH_RAW_STATE, > NSP_ETH_STATE_RATE, rate, > NSP_ETH_CTRL_SET_RATE); > } > @@ -561,6 +568,6 @@ int __nfp_eth_set_speed(struct nfp_nsp *nsp, unsigned int > speed) > */ > int __nfp_eth_set_split(struct nfp_nsp *nsp, unsigned int lanes) > { > - return nfp_eth_set_bit_config(nsp, NSP_ETH_RAW_PORT, NSP_ETH_PORT_LANES, > + return NFP_ETH_SET_BIT_CONFIG(nsp, NSP_ETH_RAW_PORT, NSP_ETH_PORT_LANES, > lanes, NSP_ETH_CTRL_SET_LANES); > }
