Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop "-march=native" from HOST flags #198

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

floppym
Copy link
Contributor

@floppym floppym commented Jan 17, 2022

GCC does not support -march=native on some targets (ia64, riscv).
The performance enhancement for makeguids isn't worth the trouble it
causes.

Bug: https://bugs.gentoo.org/831334
Signed-off-by: Mike Gilbert [email protected]

GCC does not support -march=native on some targets (ia64, riscv).
The performance enhancement for makeguids isn't worth the trouble it
causes.

Bug: https://bugs.gentoo.org/831334
Signed-off-by: Mike Gilbert <[email protected]>
Copy link
Member

@frozencemetery frozencemetery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code you're deleting already handles ia64 correctly. I am unconvinced that the right thing is to delete it wholesale rather than adding handling for riscv.

@floppym
Copy link
Contributor Author

floppym commented Jan 17, 2022

What is the purpose behind passing -march=native in the first place?

@floppym
Copy link
Contributor Author

floppym commented Jan 17, 2022

My argument:
- HOSTCC should produce binaries that run on the build system by default, without any -march flags.
- I assume -march=native is simply there to give some performance boost. This would be a silly micro-optimization, since makeguids is a relatively simple program that runs only during the build process.

@floppym
Copy link
Contributor Author

floppym commented Jan 17, 2022

It looks like -march=native was added in 8134619.

A better solution now would be to not add CPPFLAGS from the environment to HOST_CPPFLAGS.

@floppym
Copy link
Contributor Author

floppym commented Jan 17, 2022

I added a commit to stop adding CPPFLAGS/CFLAGS to HOST_CPPFLAGS/HOST_CFLAGS.

When cross compiling, these variables may contain flags that are not
compatible with both toolchains.

Signed-off-by: Mike Gilbert <[email protected]>
@frozencemetery
Copy link
Member

A better solution now would be to not add CPPFLAGS from the environment to HOST_CPPFLAGS.

I haven't thought this through fully, but I think we rely on that for package builds to inherit distro flags?

@floppym
Copy link
Contributor Author

floppym commented Jun 8, 2022

Distro packages can set HOST_CPPFLAGS if they really want to. It seems unimportant for a binary that only gets used during the build process and isn't installed by the resulting package.

@floppym
Copy link
Contributor Author

floppym commented Feb 12, 2023

Please reconsider merging this.

@asheplyakov
Copy link

It looks like -march=native was added in 8134619.

Currently makeguids is compiled with the same flags/settings as the rest
of the package, which does not work in case of cross-compiles when arch
of the build host and the target host are different.

-march=native does not influence the architecture of the binaries produced by the compiler. I.e. for GCC the target architecture is hard-coded into the binary, and for clang it can be specified by --target=arch-vendor-os-abi.

@floppym
Copy link
Contributor Author

floppym commented Mar 1, 2023

-march=native does not influence the architecture of the binaries produced by the compiler.

GCC does not support -march=native for all targets. For example, if you try to build efivar using a RISCV GCC toolchain, you the error demonstrated in this bug report.

gcc -O2 -pipe -mabi=lp64d -std=gnu11 -funsigned-char -fvisibility=hidden -specs=/var/tmp/portage/sys-libs/efivar-38/work/efivar-38/src/include/gcc.specs -fno-merge-constants  -std=gnu11 -funsigned-char -fvisibility=hidden -specs=/var/tmp/portage/sys-libs/efivar-38/work/efivar-38/src/include/gcc.specs -fno-merge-constants   -fPIC  -DLIBEFIVAR_VERSION=38 -D_GNU_SOURCE -I/var/tmp/portage/sys-libs/efivar-38/work/efivar-38/src/include/ -DEFIVAR_BUILD_ENVIRONMENT -march=native -c -o util.o util.c
gcc: error: ‘-march=native’: ISA string must begin with rv32 or rv64

There is no valid reason for adding -march=native to HOST_CPPFLAGS.

It also makes no sense to add CFLAGS and CPPFLAGS to HOST_CFLAGS and HOST_CPPFLAGS.

@dtor
Copy link
Contributor

dtor commented May 15, 2023

It looks like -march=native was added in 8134619.

Currently makeguids is compiled with the same flags/settings as the rest
of the package, which does not work in case of cross-compiles when arch
of the build host and the target host are different.

-march=native does not influence the architecture of the binaries produced by the compiler. I.e. for GCC the target architecture is hard-coded into the binary, and for clang it can be specified by --target=arch-vendor-os-abi.

I should have said "microarchitecture" not "architecture". The problem I tried to solve is that makeguids built with "-march=skylake" target that we used to build everything (not only efivars, but all packages) that is supposed to be run on a Chromebook (at that time) would trap on my workstation that had older Xeons.

[544498.193844] traps: makeguids[22888] trap invalid opcode ip:401df5 sp:7ffdc2469440 error:0 in makeguids[400000+4000]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants