This patch broke the buildkite bot
https://buildkite.com/llvm-project/upstream-bazel/builds/56515#0186dfcd-e044-4adf-992d-b51e688544dd

Thx for the heads up / fix @tstellar !

• Disable layering check when using mpfr_system
• Remove layering_check on MPRFUtils instead

• Disable layering check when using mpfr_system

Thx for the review.

rebase, rename test and retitle commit

In rG7a282bd2aaa5c1337023578426b15c294eb274bc#1175678, @aaronmondal wrote:

My bad, thanks!

In D143913#4123628, @michaelrj wrote:

The flags for my pieces don't have cmake options because I didn't know how we were going to organize them at first.

For printf:
Disable float, disable index mode, and disable write int are all independent flags. They can have simple options and I think they should default to off, off, and on respectively (so that we only disable write int).
Index arr len is takes an int, but it's already got a default in printf_config.h so its cmake options should probably default to off.
Conv atlas is intended to let users completely replace the conversion logic, since it defines where the header file that includes all the headers that define the converters is. I'd say it probably shouldn't have a cmake option at all, since the cmake targets that define the header libraries for the conversions would also need to be changed. Basically, it's for people who are using their own build system and want to use their own conversions.

For scanf:
Disable float and index mode are the same as in printf.

For strtofloat:
Scoping those options seems like a good idea. They can similarly have simple options, although it might be good to have a warning pop up when disabling simple decimal conversion since that means there's no guarantee of correctness.

• rename LLVM_LIBC_IMPLEMENTATION_DEFINED_TEST_BEHAVIOR as well
• Scope str_to_float macros and add a warning

@phosek @mcgrathr this patch would also rename LLVM_LIBC_TEST_USE_FUCHSIA to LIBC_COPT_TEST_USE_FUCHSIA to conform to our guidelines.
Let me know if that's an issue for you.

@michaelrj it seems that the following definitions are currently not exposed to the build system.
LIBC_COPT_PRINTF_CONV_ATLAS
LIBC_COPT_PRINTF_DISABLE_FLOAT
LIBC_COPT_PRINTF_DISABLE_INDEX_MODE
LIBC_COPT_PRINTF_DISABLE_WRITE_INT
LIBC_COPT_PRINTF_INDEX_ARR_LEN
LIBC_COPT_PRINTF_CONV_ATLAS
LIBC_COPT_SCANF_DISABLE_FLOAT
LIBC_COPT_SCANF_DISABLE_INDEX_MODE
LIBC_COPT_DISABLE_CLINGER_FAST_PATH
LIBC_COPT_DISABLE_EISEL_LEMIRE
LIBC_COPT_DISABLE_SIMPLE_DECIMAL_CONVERSION

• Keep CMake option as is, only change preprocessor

In D143678#4118904, @GMNGeoffrey wrote:

Looks like there's still an issue with mpfr.

One advantage to adding at the package level is that it will also apply for downstream projects using LLVM. So if someone is building via their own project and editing an LLVM submodule or something then they would still have layering enforced. Just a thought

• Fix typo

Thx a lot Tue, looks like I messed up with the FMA flags sorry.

Done

• address comments

rebase and move layering check as a global option

In D143678#4117090, @rupprecht wrote:

In D143678#4116440, @gchatelet wrote:

I just looked at the premerge checks and I can see how this will be hard to fix globally (if at all possible).
I'll use the package feature instead.

IMO we should try to clean those up, and then once we do so, remove the package-level annotations and have the feature globally enabled -- but as build --features=layering_check, not build:ci --features=layering_check.

If there are any that are really problematic, we can add a feature to disable layering check for that rule (or package)

I just looked at the premerge checks and I can see how this will be hard to fix globally (if at all possible).
I'll use the package feature instead.

In D143678#4116423, @GMNGeoffrey wrote:

layering_check has already been turned on at the package level for the packages that are layering clean (e.g. https://github.com/llvm/llvm-project/blob/main/utils/bazel/llvm-project-overlay/llvm/BUILD.bazel#L15). You could add more

In D143386#4107144, @efriedma wrote:

Almost all the specifications you're specifying are wrong. Very few targets should be using "i". Off the top of my head, 32-bit ARM, MIPS targets with MicroMIPS, and certain PowerPC targets should, but most common targets shouldn't. ("i" means that either the target mangles the bottom bits of function pointers, or function pointers don't point directly at code.)

Ok so I misunderstood the meaning of "i" then.