On Thu, May 14, 2020 at 01:05:37AM +0900, Masahiro Yamada wrote: > Hi Tom, > > On Wed, May 13, 2020 at 11:42 PM Tom Rini <tr...@konsulko.com> wrote: > > > > On Tue, May 12, 2020 at 11:04:38PM -0400, Tom Rini wrote: > > > On Mon, May 11, 2020 at 09:08:03PM +0200, Heinrich Schuchardt wrote: > > > > On 5/11/20 8:40 PM, Tom Rini wrote: > > > > > On Sun, May 10, 2020 at 10:12:07PM +0900, Masahiro Yamada wrote: > > > > >> On Sun, May 10, 2020 at 12:12 AM Heinrich Schuchardt > > > > >> <xypron.g...@gmx.de> wrote: > > > > >>> > > > > >>> GCC recognizes /* fallthrough */ if -Wimplicit-fallthrough=3 is > > > > >>> enabled. > > > > >> > > > > >> FYI. > > > > >> > > > > >> Linux decided to not use /* fallthrough */ any more > > > > >> because Clang does not recognize it. > > > > >> > > > > >> __attribute__((__fallthrough__)) is supported > > > > >> by both Clang and recent GCC. > > > > In fact Linux has a define: > > > > > > > > include/linux/compiler_attributes.h:200:# define fallthrough > > > > __attribute__((__fallthrough__)) > > > > > > > > And in the code you would use > > > > > > > > case foo: > > > > fallthrough; > > > > case bar: > > > > > > > > But the Linux kernel still has a lot of lines with > > > > > > > > /* fallthrough */ > > > > > > > > Documentation/process/deprecated.rst: > > > > > > > > <cite> > > > > As there have been a long list of flaws `due to missing "break" > > > > statements <https://cwe.mitre.org/data/definitions/484.html>`_, we no > > > > longer allow implicit fall-through. In order to identify intentional > > > > fall-through cases, we have adopted a pseudo-keyword macro "fallthrough" > > > > which expands to gcc's extension `__attribute__((__fallthrough__)) > > > > <https://gcc.gnu.org/onlinedocs/gcc/Statement-Attributes.html>`_. (When > > > > the C17/C18 `[[fallthrough]]` syntax is more commonly supported by C > > > > compilers, static analyzers, and IDEs, we can switch to using that > > > > syntax for the macro pseudo-keyword.) > > > > </cite> > > > > > > > > Using the attribute is not standard C and not any better than using the > > > > comment. The real target is the C17 syntax. > > > > > > > > >> > > > > >> > > > > >> Linux is now doing treewide conversion > > > > >> from /* fallthrough */ to 'fallthrough;'. > > > > >> > > > > >> See include/linux/compiler_attributes.h in Linux. > > > > >> > > > > >> I do not know if U-Boot wants to align with it. > > > > >> (up to Tom ?) > > > > > > > > > > A re-sync on the compiler headers again and making use of this sounds > > > > > like a good idea, yes. > > > > > > > > > > > > > We should enable -Wimplicit-fallthrough like the kernel does. This > > > > defaults to -Wimplicit-fallthrough=3 and is happy with both the comment > > > > as well as with the attribute. > > > > > > > > @Tom: > > > > Will you update the compiler headers within this release cycle? > > > > Otherwise we should take the patch as is to get us closer to the > > > > -Wimplicit-fallthrough target. > > > > > > I'm not going to update it for this release cycle. I've done the > > > initial import and build and there's some fairly large changes related > > > to inlining that I want to look at harder to see if we can/should do > > > something about (I don't want to derail this thread, I'll start > > > another). But it's very far from zero size change and given the inline > > > changes I think it'll need real testing. > > > > > > And since the kernel isn't making a huge use yet of fallthrough; we can > > > afford to look a little harder at things. > > > > I think I've figured out the inline issue which is that we need > > scripts/Kconfig.include from the kernel, CC_HAS_ASM_INLINE Kconfig > > option, and re-sync with Kconfiglib, but that's still going to be enough > > stuff that I don't think it's good to pull in at -rc2. > > > > > I do not get how 'asm inline' support is related > to this topic. > > GCC 9 started to support 'asm inline' for the better inlining heuristic. > The kernel uses a bunch of inline assembly > that is not as expensive as it looks. > > As GCC is agnostic about the real cost of inline assembly, > 'asm inline' is a good hint if people know the real cost is quite small. > Then, GCC will be able to inline more functions. > > I do not know how important it is for U-Boot, though. > > What is causing you a trouble?
So, it turns out that while we do want to grab the changes so that we
can have CC_HAS_ASM_INLINE via Kconfig, it's not "it". What I see for
virtually every board (with gcc-9.3 from kernel.org) is something like:
rock960-rk3399 : all -8 rodata -4 spl/u-boot-spl:all +992
spl/u-boot-spl:text +992 text -4
u-boot: add: 67/-9, grow: 74/-92 bytes: 5072/-4928 (144)
function old new delta
static._compare_and_overwrite_entry - 348 +348
menu_interactive_choice - 288 +288
hex2bin - 200 +200
__fswab64 - 176 +176
__fswab32 - 144 +144
sdhci_reset - 136 +136
dwmci_fifo_ready - 124 +124
fdt_offset_ptr_ - 120 +120
menu_items_iter - 108 +108
generic_fls - 100 +100
fdt_set_totalsize - 96 +96
static.generic_fls - 84 +84
clk_get_by_indexed_prop - 80 +80
fdt_read_number - 76 +76
do_fdt 3984 4060 +76
usb_kbd_poll_for_event - 72 +72
rpc_lookup_req - 72 +72
menu_item_key_match - 72 +72
bin2hex - 68 +68
asix_get_phy_addr - 68 +68
fdt_setprop_u64 - 64 +64
fdt_set_size_dt_strings - 64 +64
fdt_set_off_dt_strings - 64 +64
zalloc - 60 +60
static.strlcat - 60 +60
dwmci_wait_reset - 60 +60
menu_item_print - 56 +56
is_zero_ethaddr - 56 +56
asix_eth_start 228 284 +56
set_sctlr - 52 +52
menu_item_destroy - 52 +52
get_sctlr - 48 +48
fdt_setprop_u32 - 48 +48
is_valid_ethaddr - 44 +44
is_hex_prefix - 44 +44
fdt_data_size_ - 44 +44
list_del - 40 +40
fdt_mem_rsv_ - 40 +40
dev_read_u32_default - 40 +40
__tolower - 40 +40
le32_to_int - 36 +36
fdt_open_into 392 428 +36
_debug_uart_putc - 36 +36
usb_gadget_disconnect - 32 +32
usb_gadget_connect - 32 +32
fdt_set_size_dt_struct - 32 +32
fdt_set_off_dt_struct - 32 +32
fdt_packblocks_ 176 208 +32
fdt_blocks_misordered_ 96 128 +32
__get_unaligned_le32 - 32 +32
fdtdec_get_number 48 76 +28
fdt_ro_probe_ 128 156 +28
env_flags_validate 632 660 +28
clk_valid - 28 +28
clk_set_rate 52 80 +28
clk_set_parent 52 80 +28
clk_get_rate 52 80 +28
clk_free 44 72 +28
clk_enable 52 80 +28
clk_disable 52 80 +28
bootstage_error - 28 +28
asix_set_sw_mii - 28 +28
asix_set_hw_mii - 28 +28
uuid_str_to_bin 396 420 +24
ofnode_read_u64 92 116 +24
fdt_mem_rsv 60 84 +24
fdt_get_property_namelen 44 68 +24
static.usb_ep_queue - 20 +20
static.image_get_size - 20 +20
is_extended - 20 +20
flush_dcache_all - 20 +20
fdt_splice_mem_rsv_ 96 116 +20
fdt_offset_ptr 104 124 +20
fdt_check_header 276 296 +20
eth_validate_ethaddr_str 152 172 +20
android_image_get_kcomp 84 104 +20
usb_ep_free_request - 16 +16
static.image_get_magic - 16 +16
static.image_get_load - 16 +16
nfs_read_req 228 244 +16
malloc_cache_aligned - 16 +16
image_print_contents 448 464 +16
fit_get_end 16 32 +16
fdt_add_property_ 388 404 +16
dwc3_flush_cache - 16 +16
do_bootm_states 2376 2392 +16
dev_read_bool - 16 +16
static.image_get_ep - 12 +12
static.dev_read_u32_default - 12 +12
nfs_readlink_reply 336 348 +12
nfs_read_reply 404 416 +12
fit_image_get_address 132 144 +12
fdtdec_parse_phandle_with_args 336 348 +12
fdt_shrink_to_minimum 220 232 +12
fdt_header_size 12 24 +12
fdt_del_node 96 108 +12
fdt_add_mem_rsv 124 136 +12
boot_get_fdt 1028 1040 +12
android_image_get_kernel 572 584 +12
update_package_list 280 288 +8
nfs_lookup_reply 292 300 +8
net_copy_u32 - 8 +8
net_copy_ip - 8 +8
image_multi_getimg 132 140 +8
image_check_dcrc 68 76 +8
guidcmp - 8 +8
genimg_get_format 92 100 +8
free_packagelist 68 76 +8
fit_image_load 1608 1616 +8
fdt_valid 204 212 +8
fdt_splice_struct_ 96 104 +8
fdt_rw_probe_ 108 116 +8
fdt_num_mem_rsv 60 68 +8
fdt_next_tag 256 264 +8
fdt_getprop_namelen 100 108 +8
dhcp_extended 616 624 +8
boot_get_ramdisk 964 972 +8
xhci_submit_control_msg 2732 2736 +4
static.image_get_hcrc - 4 +4
static.image_get_dcrc - 4 +4
static.__swab32p - 4 +4
sd_get_capabilities 432 436 +4
rpc_req 288 292 +4
rpc_lookup_reply 180 184 +4
print_data 444 448 +4
nfs_umountall_reply 136 140 +4
nfs_readlink_req 192 196 +4
nfs_mount_req 180 184 +4
nfs_mount_reply 148 152 +4
nfs_lookup_req 324 328 +4
image_setup_libfdt 284 288 +4
image_check_hcrc 80 84 +4
fdtdec_get_int_array 96 100 +4
fdt_setprop_placeholder 216 220 +4
fdt_getprop_by_offset 184 188 +4
fdt_get_string 288 292 +4
fdt_get_property_namelen_ 212 216 +4
fdt_get_property_by_offset_ 100 104 +4
fdt_get_phandle 120 124 +4
fdt_get_mem_rsv 108 112 +4
boot_relocate_fdt 424 428 +4
spi_slave_ofdata_to_platdata 432 428 -4
sdhci_send_command 1660 1656 -4
i2c_chip_ofdata_to_platdata 100 96 -4
file_fat_write_at 652 648 -4
fdt_get_name 164 160 -4
fat_size 204 200 -4
fat_opendir 172 168 -4
fat_exists 128 124 -4
efi_search_protocol 148 144 -4
efi_install_multiple_protocol_interfaces 552 548
-4
dwmci_send_cmd 1556 1552 -4
remove_strings_package 124 116 -8
fdt_splice_ 148 140 -8
fdt_pack_reg 216 208 -8
fdt_add_subnode_namelen 284 276 -8
fastboot_start_ep 124 116 -8
efi_signal_event 200 192 -8
efi_process_event_queue 144 136 -8
efi_install_fdt 868 860 -8
efi_install_configuration_table 456 448 -8
efi_disconnect_all_drivers 404 396 -8
clk_set_defaults 516 508 -8
ustrtoull 144 132 -12
ustrtoul 144 132 -12
rx_handler_dl_image 204 192 -12
rx_handler_command 368 356 -12
remove_keyboard_package 76 64 -12
regulator_common_ofdata_to_platdata 188 176 -12
read_bootsectandvi 308 296 -12
free_keyboard_layouts 80 68 -12
file_fat_read_at 864 852 -12
fat_mkdir 928 916 -12
fat_itr_root 444 432 -12
fastboot_tx_write_str 152 140 -12
fastboot_set_alt 316 304 -12
efi_uninstall_protocol_interface 112 100 -12
efi_uninstall_protocol 216 204 -12
efi_uninstall_multiple_protocol_interfaces 464 452
-12
efi_remove_protocol 100 88 -12
efi_locate_handle 380 368 -12
efi_exit_boot_services 336 324 -12
efi_delete_handle 60 48 -12
efi_close_protocol 220 208 -12
dwc3_prepare_trbs 772 760 -12
dwc3_gadget_uboot_handle_interrupt 1552 1540 -12
dwc3_gadget_giveback 240 228 -12
g_dnl_bind 376 360 -16
fastboot_disable 140 124 -16
ext4fs_iterate_dir 656 640 -16
efi_locate_protocol 280 264 -16
efi_add_protocol 320 304 -16
dhcp_process_options 512 496 -16
fat_unlink 668 648 -20
ext4fs_read_inode 392 372 -20
ext4fs_mount 236 216 -20
dhcp_handler 972 952 -20
_parse_integer_fixup_radix 248 228 -20
g_dnl_unbind 52 28 -24
fdt_initrd 436 412 -24
efi_hii_package_len 24 - -24
dcache_status 52 24 -28
usb_kbd_testc 144 112 -32
usb_kbd_getc 116 84 -32
ext4fs_find_file1 660 628 -32
asix_eth_probe 700 668 -32
dwc3_gadget_ep_enable 264 228 -36
printch 80 40 -40
eth_write_hwaddr 220 180 -40
efi_close_event 268 224 -44
asix_mdio_write 140 96 -44
add_packages 928 884 -44
of_bus_default_map 168 120 -48
menu_destroy 128 80 -48
of_bus_default_translate 184 132 -52
mmc_init 2724 2668 -56
static.dwmci_wait_reset 60 - -60
__of_translate_address 624 564 -60
remove_guid_package 64 - -64
menu_item_add 240 176 -64
menu_default_set 148 76 -72
icache_disable 100 24 -76
static.clk_get_by_indexed_prop 80 - -80
dcache_disable 132 48 -84
icache_enable 116 28 -88
nfs_send 260 168 -92
xhci_microframes_to_exponent 104 - -104
skip_num 104 - -104
spi_nor_scan 2196 2088 -108
part_get_info_extended 716 604 -112
static.dwmci_fifo_ready 124 - -124
static.sdhci_reset 136 - -136
print_partition_extended 648 512 -136
asix_mdio_read 140 - -140
static.parse_attr 468 308 -160
efi_set_variable_common 1440 1276 -164
efi_get_variable_common 548 384 -164
eth_post_probe 580 404 -176
dcache_enable 272 44 -228
read_allocated_block 2120 1832 -288
hsearch_r 1080 728 -352
menu_get_choice 480 84 -396
spl-u-boot-spl: add: 23/-5, grow: 25/-12 bytes: 1700/-768 (932)
function old new delta
sdhci_reset - 136 +136
__fswab64 - 132 +132
dwmci_fifo_ready - 124 +124
fdt_offset_ptr_ - 120 +120
__fswab32 - 104 +104
fdt_mem_rsv_ - 80 +80
clk_get_by_indexed_prop - 80 +80
fdt_set_size_dt_strings - 64 +64
fdt_set_off_dt_strings - 64 +64
dwmci_wait_reset - 60 +60
set_sctlr - 52 +52
get_sctlr - 48 +48
fdt_setprop_u32 - 48 +48
fdt_data_size_ - 44 +44
__tolower - 40 +40
_debug_uart_putc - 36 +36
fdt_set_size_dt_struct - 32 +32
fdt_set_off_dt_struct - 32 +32
fdtdec_get_number 48 76 +28
fdt_offset_ptr 52 80 +28
clk_valid - 28 +28
clk_set_rate 52 80 +28
clk_set_parent 52 80 +28
clk_get_rate 52 80 +28
ofnode_read_u64 92 116 +24
flush_dcache_all - 20 +20
fdt_splice_mem_rsv_ 96 116 +20
fdt_check_header 28 44 +16
dev_read_u32_default - 16 +16
dev_read_bool - 16 +16
fit_image_get_address 132 144 +12
fdtdec_parse_phandle_with_args 336 348 +12
fdt_shrink_to_minimum 220 232 +12
fdt_get_property_by_offset_ 36 48 +12
fdt_add_property_ 340 352 +12
fdt_splice_struct_ 96 104 +8
fdt_get_string 64 72 +8
fdt_add_mem_rsv 112 120 +8
static.__swab32p - 4 +4
sd_get_capabilities 432 436 +4
fdtdec_get_int_array 96 100 +4
fdt_setprop_placeholder 196 200 +4
fdt_num_mem_rsv 64 68 +4
fdt_next_tag 192 196 +4
fdt_getprop_by_offset 80 84 +4
fdt_get_property_namelen_ 200 204 +4
fdt_get_phandle 120 124 +4
fdt_get_mem_rsv 52 56 +4
sdhci_send_command 1660 1656 -4
fdt_del_mem_rsv 104 100 -4
dwmci_send_cmd 1556 1552 -4
fdt_splice_ 148 140 -8
fdt_add_subnode_namelen 260 252 -8
fdt_get_name 68 56 -12
clk_set_defaults 488 472 -16
fdt_mem_rsv 20 - -20
_parse_integer_fixup_radix 248 228 -20
fdt_record_loadable 372 336 -36
printch 80 40 -40
static.dwmci_wait_reset 60 - -60
static.clk_get_by_indexed_prop 80 - -80
dcache_disable 132 48 -84
mmc_init 4576 4464 -112
static.dwmci_fifo_ready 124 - -124
static.sdhci_reset 136 - -136
Which is not good, and I need to dig in to a bit more to see what
changes cause this.
--
Tom
signature.asc
Description: PGP signature
