* [PATCH] igb: Convert a series of if statements to switch case
@ 2022-05-11 9:20 xiaolinkui
2022-05-12 13:14 ` David Laight
0 siblings, 1 reply; 5+ messages in thread
From: xiaolinkui @ 2022-05-11 9:20 UTC (permalink / raw)
To: pmenzel, jesse.brandeburg, anthony.l.nguyen
Cc: davem, edumazet, kuba, pabeni, intel-wired-lan, netdev,
Linkui Xiao
From: Linkui Xiao <xiaolinkui@kylinos.cn>
Convert a series of if statements that handle different events to a switch
case statement to simplify the code.
V2: fix patch description and email format.
Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
---
drivers/net/ethernet/intel/igb/igb_main.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 34b33b21e0dc..4ce0718eeff6 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -4588,13 +4588,17 @@ static inline void igb_set_vf_vlan_strip(struct igb_adapter *adapter,
struct e1000_hw *hw = &adapter->hw;
u32 val, reg;
- if (hw->mac.type < e1000_82576)
+ switch (hw->mac.type) {
+ case e1000_undefined:
+ case e1000_82575:
return;
-
- if (hw->mac.type == e1000_i350)
+ case e1000_i350:
reg = E1000_DVMOLR(vfn);
- else
+ break;
+ default:
reg = E1000_VMOLR(vfn);
+ break;
+ }
val = rd32(reg);
if (enable)
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* RE: [PATCH] igb: Convert a series of if statements to switch case
2022-05-11 9:20 [PATCH] igb: Convert a series of if statements to switch case xiaolinkui
@ 2022-05-12 13:14 ` David Laight
2022-05-12 13:31 ` Linkui Xiao
2022-05-12 18:47 ` Tony Nguyen
0 siblings, 2 replies; 5+ messages in thread
From: David Laight @ 2022-05-12 13:14 UTC (permalink / raw)
To: 'xiaolinkui', pmenzel@molgen.mpg.de,
jesse.brandeburg@intel.com, anthony.l.nguyen@intel.com
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, intel-wired-lan@lists.osuosl.org,
netdev@vger.kernel.org, Linkui Xiao
> From: Linkui Xiao <xiaolinkui@kylinos.cn>
>
> Convert a series of if statements that handle different events to a switch
> case statement to simplify the code.
>
> V2: fix patch description and email format.
>
> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
> ---
> drivers/net/ethernet/intel/igb/igb_main.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index 34b33b21e0dc..4ce0718eeff6 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -4588,13 +4588,17 @@ static inline void igb_set_vf_vlan_strip(struct igb_adapter *adapter,
> struct e1000_hw *hw = &adapter->hw;
> u32 val, reg;
>
> - if (hw->mac.type < e1000_82576)
> + switch (hw->mac.type) {
> + case e1000_undefined:
> + case e1000_82575:
> return;
> -
> - if (hw->mac.type == e1000_i350)
> + case e1000_i350:
> reg = E1000_DVMOLR(vfn);
> - else
> + break;
> + default:
> reg = E1000_VMOLR(vfn);
> + break;
> + }
>
> val = rd32(reg);
> if (enable)
> --
> 2.17.1
Are you sure that generates reasonable code?
The compiler could generate something completely different
for the two versions.
It isn't even obvious they are equivalent.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] igb: Convert a series of if statements to switch case
2022-05-12 13:14 ` David Laight
@ 2022-05-12 13:31 ` Linkui Xiao
2022-05-12 18:47 ` Tony Nguyen
1 sibling, 0 replies; 5+ messages in thread
From: Linkui Xiao @ 2022-05-12 13:31 UTC (permalink / raw)
To: David Laight, pmenzel@molgen.mpg.de, jesse.brandeburg@intel.com,
anthony.l.nguyen@intel.com
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, intel-wired-lan@lists.osuosl.org,
netdev@vger.kernel.org, Linkui Xiao
Dear David,
Thanks for your reply
Logically the two versions are equivalent, hw->mac.type is defined as
follows:
enum e1000_mac_type {
e1000_undefined = 0,
e1000_82575,
e1000_82576,
e1000_82580,
e1000_i350,
e1000_i354,
e1000_i210,
e1000_i211,
e1000_num_macs /* List is 1-based, so subtract 1 for true
count. */
};
Therefore, hw->mac.type < e1000_82576 has only two cases:
e1000_undefined or e1000_82575.
On 5/12/22 21:14, David Laight wrote:
>> From: Linkui Xiao <xiaolinkui@kylinos.cn>
>>
>> Convert a series of if statements that handle different events to a switch
>> case statement to simplify the code.
>>
>> V2: fix patch description and email format.
>>
>> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
>> ---
>> drivers/net/ethernet/intel/igb/igb_main.c | 12 ++++++++----
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
>> index 34b33b21e0dc..4ce0718eeff6 100644
>> --- a/drivers/net/ethernet/intel/igb/igb_main.c
>> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
>> @@ -4588,13 +4588,17 @@ static inline void igb_set_vf_vlan_strip(struct igb_adapter *adapter,
>> struct e1000_hw *hw = &adapter->hw;
>> u32 val, reg;
>>
>> - if (hw->mac.type < e1000_82576)
>> + switch (hw->mac.type) {
>> + case e1000_undefined:
>> + case e1000_82575:
>> return;
>> -
>> - if (hw->mac.type == e1000_i350)
>> + case e1000_i350:
>> reg = E1000_DVMOLR(vfn);
>> - else
>> + break;
>> + default:
>> reg = E1000_VMOLR(vfn);
>> + break;
>> + }
>>
>> val = rd32(reg);
>> if (enable)
>> --
>> 2.17.1
> Are you sure that generates reasonable code?
> The compiler could generate something completely different
> for the two versions.
>
> It isn't even obvious they are equivalent.
>
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] igb: Convert a series of if statements to switch case
2022-05-12 13:14 ` David Laight
2022-05-12 13:31 ` Linkui Xiao
@ 2022-05-12 18:47 ` Tony Nguyen
1 sibling, 0 replies; 5+ messages in thread
From: Tony Nguyen @ 2022-05-12 18:47 UTC (permalink / raw)
To: David Laight, 'xiaolinkui', pmenzel@molgen.mpg.de,
jesse.brandeburg@intel.com
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, intel-wired-lan@lists.osuosl.org,
netdev@vger.kernel.org, Linkui Xiao
On 5/12/2022 6:14 AM, David Laight wrote:
>> From: Linkui Xiao <xiaolinkui@kylinos.cn>
>>
>> Convert a series of if statements that handle different events to a switch
>> case statement to simplify the code.
>>
>> V2: fix patch description and email format.
>>
>> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
>> ---
>> drivers/net/ethernet/intel/igb/igb_main.c | 12 ++++++++----
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
>> index 34b33b21e0dc..4ce0718eeff6 100644
>> --- a/drivers/net/ethernet/intel/igb/igb_main.c
>> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
>> @@ -4588,13 +4588,17 @@ static inline void igb_set_vf_vlan_strip(struct igb_adapter *adapter,
>> struct e1000_hw *hw = &adapter->hw;
>> u32 val, reg;
>>
>> - if (hw->mac.type < e1000_82576)
>> + switch (hw->mac.type) {
>> + case e1000_undefined:
>> + case e1000_82575:
>> return;
>> -
>> - if (hw->mac.type == e1000_i350)
>> + case e1000_i350:
>> reg = E1000_DVMOLR(vfn);
>> - else
>> + break;
>> + default:
>> reg = E1000_VMOLR(vfn);
>> + break;
>> + }
>>
>> val = rd32(reg);
>> if (enable)
>> --
>> 2.17.1
>
> Are you sure that generates reasonable code?
> The compiler could generate something completely different
> for the two versions.
>
> It isn't even obvious they are equivalent.
It seems like these just happen to line up this way and appear to be two
different checks; one as the bail out for unsupported MACs and the other
for register reads for supported ones. Combining them seems to muddy
that distinction.
Thanks,
Tony
>
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] igb: Convert a series of if statements to switch case
@ 2022-05-10 2:57 xiaolinkui
0 siblings, 0 replies; 5+ messages in thread
From: xiaolinkui @ 2022-05-10 2:57 UTC (permalink / raw)
To: jesse.brandeburg, anthony.l.nguyen
Cc: davem, edumazet, kuba, pabeni, intel-wired-lan, netdev,
Linkui Xiao
From: Linkui Xiao<xiaolinkui@kylinos.cn>
Convert a series of if statements that handle different events to
a switch case statement to simplify the code.
Signed-off-by: Linkui Xiao<xiaolinkui@kylinos.cn>
---
drivers/net/ethernet/intel/igb/igb_main.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 34b33b21e0dc..4ce0718eeff6 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -4588,13 +4588,17 @@ static inline void igb_set_vf_vlan_strip(struct igb_adapter *adapter,
struct e1000_hw *hw = &adapter->hw;
u32 val, reg;
- if (hw->mac.type < e1000_82576)
+ switch (hw->mac.type) {
+ case e1000_undefined:
+ case e1000_82575:
return;
-
- if (hw->mac.type == e1000_i350)
+ case e1000_i350:
reg = E1000_DVMOLR(vfn);
- else
+ break;
+ default:
reg = E1000_VMOLR(vfn);
+ break;
+ }
val = rd32(reg);
if (enable)
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-12 18:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-11 9:20 [PATCH] igb: Convert a series of if statements to switch case xiaolinkui
2022-05-12 13:14 ` David Laight
2022-05-12 13:31 ` Linkui Xiao
2022-05-12 18:47 ` Tony Nguyen
-- strict thread matches above, loose matches on Subject: below --
2022-05-10 2:57 xiaolinkui
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).