BuildMode

[David_A_Greene1]

I'm continuing my quest to integrate _GLIBCXX_DEBUG into the config system.
I now have things at a point where I can configure llvm to build with
-D_GLIBCXX_DEBUG and the llvm-gcc will pick up the correct CPPFLAGS
automatically.

One thing I noticed in the llvm Makefile.rules is this:

# If DISABLE_ASSERTIONS=1 is specified (make command line or configured),
# then disable assertions by defining the appropriate preprocessor symbols.
ifdef DISABLE_ASSERTIONS
  BuildMode := $(BuildMode)-Asserts
[...]
else
[...]
endif

This seems backward to make. If I configure with --disable-assertions I would
expect the build to be in a directory called Debug. If I --enable-asserts
(the default), I would expect it in Debug-Asserts. But this logic does just
the opposite. So I changed it in my copy to do what I expected (and changed
the llvm-gcc configuration correspondingly).

Is this what was intended and the above code is a bug or did I misinterpret
something?

Now in the llvm Makefile:

# NOTE: This needs to remain as the last target definition in this file so
# that it gets executed last.
all::
  $(Echo) '*****' Completed $(BuildMode)$(AssertMode) Build
ifeq ($(BuildMode),Debug)
  $(Echo) '*****' Note: Debug build can be 10 times slower than an
  $(Echo) '*****' optimized build. Use 'make ENABLE_OPTIMIZED=1' to
  $(Echo) '*****' make an optimized build.
endif

This is the only place I can find AssertMode mentioned. I would much rather
keep things specified in BuildMode. I add "-ExpensiveChecks" to BuildMode
when --enable-expensive-checks is passed to configure (it is off by default).
This is what turns on -D_GLIBCXX_DEBUG. That way I can keep around Debug
builds with and without expensive run-time checks.

In summary, it looks like the build system is a little confused and perhaps
some changes were started in the past but never completed (AssertMode,
etc.). I tried to clean things up to be consistent and straightforward. Does
it look ok to everyone?

                                            -Dave

[Reid_Spencer]

Hi David,

I'm continuing my quest to integrate _GLIBCXX_DEBUG into the config system.
I now have things at a point where I can configure llvm to build with
-D_GLIBCXX_DEBUG and the llvm-gcc will pick up the correct CPPFLAGS
automatically.

Okay.

One thing I noticed in the llvm Makefile.rules is this:

# If DISABLE_ASSERTIONS=1 is specified (make command line or configured),
# then disable assertions by defining the appropriate preprocessor symbols.
ifdef DISABLE_ASSERTIONS
  BuildMode := $(BuildMode)-Asserts
[...]
else
[...]
endif

This seems backward to make. If I configure with --disable-assertions I would
expect the build to be in a directory called Debug. If I --enable-asserts
(the default), I would expect it in Debug-Asserts. But this logic does just
the opposite. So I changed it in my copy to do what I expected (and changed
the llvm-gcc configuration correspondingly).

Is this what was intended and the above code is a bug or did I misinterpret
something?

I think you mis-interpreted something. LLVM has several build modes and
while they are confusing, it makes sense if you look at the details.
Basically, there are three things that are being controlled here:
Optimization (e.g. -O2), Debug code (-D_NDEBUG) and assertions. We have
pre-defined build modes as follows:

Release = optimized + assertions
Debug = assertions + debug (not optimized)
Release-Asserts = optimized (only)
Debug-Asserts = debug (not optimized, no asserts)

If you build with -disable-assertions you are telling whatever build
mode (Debug or Release) to not include assertions. Consequently you get
either Release-Asserts (Release minus asserts) or Debug-Asserts (Debug
minus Asserts).

If you build with -enable-assertions you restating the default so you
don't get the "minus asserts" suffix.

I would appreciate it if you would not commit the changes you made or
you'll confuse a lot of us. Its already confusing enough.

Now in the llvm Makefile:

# NOTE: This needs to remain as the last target definition in this file so
# that it gets executed last.
all::
  $(Echo) '*****' Completed $(BuildMode)$(AssertMode) Build
ifeq ($(BuildMode),Debug)
  $(Echo) '*****' Note: Debug build can be 10 times slower than an
  $(Echo) '*****' optimized build. Use 'make ENABLE_OPTIMIZED=1' to
  $(Echo) '*****' make an optimized build.
endif

This is the only place I can find AssertMode mentioned. I would much rather
keep things specified in BuildMode. I add "-ExpensiveChecks" to BuildMode
when --enable-expensive-checks is passed to configure (it is off by default).
This is what turns on -D_GLIBCXX_DEBUG. That way I can keep around Debug
builds with and without expensive run-time checks.

Yeah, this appears to be bug in this rule. It shouldn't be using
$(AssertMode) there, just $(BuildMode). Furthermore, the ifeq directive
needs to find "Debug" anywhere in $(BuildMode) not just if its equal to
"Build". That is, you want the warnings to occur when BuildMOde is
"Debug" or "Debug-Asserts" or "Debug+ExpensiveChecks" or any other
variant that includes "Debug".

As a side note, you are ADDING something to the build rather than
subtracting so please use + instead of - in your BuildMode for
--enable-expensive-checks. That is, when you add
--enable-expensive-checks, the BuildMode should be "Debug
+ExpensiveChecks" not "Debug-ExpensiveChecks".

In summary, it looks like the build system is a little confused and perhaps
some changes were started in the past but never completed (AssertMode,
etc.). I tried to clean things up to be consistent and straightforward. Does
it look ok to everyone?

No. I think you're missing the point about - vs + as described above.
The way we define Build mode is as either "Release" or "Debug" followed
by things either added to or subtracted from the default configuration
for those. For example, you could have:

configure --enable-optimized --disable-assertions
--enable-expensive-checks

I would expect the build mode for this to be:

Release-Asserts+ExpensiveChecks

Reid.

[David_A_Greene]

I think you mis-interpreted something.

That's not surprising.

If you build with -disable-assertions you are telling whatever build
mode (Debug or Release) to not include assertions. Consequently you get
either Release-Asserts (Release minus asserts) or Debug-Asserts (Debug
minus Asserts).

I was wondering if that's what it meant.

I would appreciate it if you would not commit the changes you made or
you'll confuse a lot of us. Its already confusing enough.

Nope, I won't. That's why I asked. I'll change it back in my copy.

Yeah, this appears to be bug in this rule. It shouldn't be using
$(AssertMode) there, just $(BuildMode).

All right. I'll clean that up.

Furthermore, the ifeq directive
needs to find "Debug" anywhere in $(BuildMode) not just if its equal to
"Build". That is, you want the warnings to occur when BuildMOde is
"Debug" or "Debug-Asserts" or "Debug+ExpensiveChecks" or any other
variant that includes "Debug".

Yep, I wondered about that too. I'll fix it.

As a side note, you are ADDING something to the build rather than
subtracting so please use + instead of - in your BuildMode for
--enable-expensive-checks. That is, when you add
--enable-expensive-checks, the BuildMode should be "Debug
+ExpensiveChecks" not "Debug-ExpensiveChecks".

Ok. Is "ExpensiveChecks" a reasonable keyword for this? I wanted something
that could encompass other things people might want without creating
thousands of build combinations and
Debug+CxxLibChecks+BoundsChecking+ElectricFence+Even+More+
  Things+That+Would+Create+Ridiculously+Long+Build+Names

I wanted to keep things at least mostly sane.

                                             -Dave

[Reid_Spencer]

Hi David,

> I think you mis-interpreted something.

That's not surprising.

> If you build with -disable-assertions you are telling whatever build
> mode (Debug or Release) to not include assertions. Consequently you get
> either Release-Asserts (Release minus asserts) or Debug-Asserts (Debug
> minus Asserts).

I was wondering if that's what it meant.

Yeah, that's the confusing part.

> I would appreciate it if you would not commit the changes you made or
> you'll confuse a lot of us. Its already confusing enough.

Nope, I won't. That's why I asked. I'll change it back in my copy.

THanks.

> Yeah, this appears to be bug in this rule. It shouldn't be using
> $(AssertMode) there, just $(BuildMode).

All right. I'll clean that up.

Thanks.

> Furthermore, the ifeq directive
> needs to find "Debug" anywhere in $(BuildMode) not just if its equal to
> "Build". That is, you want the warnings to occur when BuildMOde is
> "Debug" or "Debug-Asserts" or "Debug+ExpensiveChecks" or any other
> variant that includes "Debug".

Yep, I wondered about that too. I'll fix it.

Thanks.

> As a side note, you are ADDING something to the build rather than
> subtracting so please use + instead of - in your BuildMode for
> --enable-expensive-checks. That is, when you add
> --enable-expensive-checks, the BuildMode should be "Debug
> +ExpensiveChecks" not "Debug-ExpensiveChecks".

Ok. Is "ExpensiveChecks" a reasonable keyword for this? I wanted something
that could encompass other things people might want without creating
thousands of build combinations and
Debug+CxxLibChecks+BoundsChecking+ElectricFence+Even+More+
  Things+That+Would+Create+Ridiculously+Long+Build+Names

Heh.

I wanted to keep things at least mostly sane.

I think its fine to group these things under one name. Although
"+Checks" would be fine with me. Its shorter and it goes along with
"-Asserts". Checks are generally understood to be expensive so
"+ExpensiveChecks" doesn't add much meaning.

We can define "+Checks" to mean whatever we want. You included
CxxLibChecks, Bounds checking, and Electric Fence. There might be other
things as well. However, I think Electric Fence should be its own
option. That is *really* expensive and implies the availability of the
EF software, etc. But, its up to you. Do what you think is right

Reid.