[PATCH 02/31] kconfig: Add support for conditional values

[Rasmus Villemoes]

On 13/01/2022 13.52, Tom Rini wrote:
> On Thu, Jan 13, 2022 at 08:56:02AM +0100, Rasmus Villemoes wrote:
>> On 12/01/2022 22.56, Tom Rini wrote:
>>> I also think I've seen cases where doing:
>>> if (CONFIG_EVALUATES_TO_ZERO) {
>>>   ...
>>> }
>>>
>>> takes more space in the binary than an #ifdef does.
>>
>> Please provide a specific example. If CONFIG_EVALUATES_TO_ZERO is any
>> integer-constant-expression evaluating at compile-time to 0, gcc throws
>> away the whole block very early during parsing. If it doesn't, that's a
>> compiler bug, so let's please not make decisions based on
>> not-even-anecdotal data.
> 
> OK.  I believe it was commit 7856cd5a6dd6 ("Convert CONFIG_SYS_PCI_64BIT
> to Kconfig") a few platforms changed size 

Can you remember which ones? I'd like to see if I can reproduce.

That said, that commit made the Kconfig symbol 'default y if PPC'. Are
you really sure all ppc-boards that set CONFIG_PCI also used to set
SYS_PCI_64BIT?

And another thing I notice is that a lot of the #define removals remove

#define CONFIG_SYS_PCI_64BIT

and not

#define CONFIG_SYS_PCI_64BIT 1

Now that doesn't matter for the places that test the definedness of
CONFIG_SYS_PCI_64BIT, because kconfig either doesn't define it or define
it with value 1. But it does matter for (the single) IS_ENABLED() use,
because IS_ENABLED(bla) evaluates to 1 if and only if bla expands to 1.
Or rather, if and only if __ARG_PLACEHOLDER_ concatenated with the
expansion of bla in turn expands to "0, " - which only happens if we hit
the __ARG_PLACEHOLDER_1 macro.

So when bla is defined with an empty expansion, for the purpose of
IS_ENABLED, it might as well not be defined or expand to 0 or to
gobbledygook.

And when one knows what to look for, it's easy to demonstrate:

$ export ARCH=ppc
$ export CROSS_COMPILE=powerpc-linux-gnu-
$ git checkout 7856cd5a6dd6~1
$ make T1042D4RDB_defconfig
$ make drivers/pci/pci-uclass.i
$ grep -C3 'beyond the 32-bit boundary' drivers/pci/pci-uclass.i

  if (!(0) &&
      type == 0x00000000 && ((u32)(((pci_addr) >> 16) >> 16))) {
   ({ if (0) printf(" - beyond the 32-bit boundary, ignoring\n"); });
   continue;
  }

$ git checkout 7856cd5a6dd6
$ make T1042D4RDB_defconfig
$ make drivers/pci/pci-uclass.i
$ grep -C3 'beyond the 32-bit boundary' drivers/pci/pci-uclass.i

  if (!(1) &&
      type == 0x00000000 && ((u32)(((pci_addr) >> 16) >> 16))) {
   ({ if (0) printf(" - beyond the 32-bit boundary, ignoring\n"); });
   continue;
  }

Whether that change makes the generated code smaller or larger I can't
say, but it's certainly not a nop semantically. [Of course, the change
is for the better, as the generated code now matches the intention;
previously 64 bit pci addresses would be ignored for the boards that had
an empty definition of CONFIG_SYS_PCI_64BIT.]

But it has nothing whatsoever to do with whether gcc is capable of
throwing away a whole "if (0)" block. But I will believe that other
Kconfig conversions have been bit by the same issue, making it _seem_
like IS_ENABLED() is somehow at fault and #ifdefs are "better".

Rasmus