Always specify -DNVFUSER_BUILD_WITH_ASAN to cmake

[wujingyue]

This might just show my ignorance of CMake. Let me know if you have a better fix.

Symptom

When I run python setup.py --build-with-asan and then python setup.py, the second build still has asan enabled.

Investigation

The first build runs cmake -DNVFUSER_BUILD_WITH_ASAN=ON, which adds NVFUSER_BUILD_WITH_ASAN:BOOL=ON to CMakeLists.txt

The second build runs cmake without -DNVFUSER_BUILD_WITH_ASAN specified. cmake chooses to use the cached value and therefore doesn't change the existing NVFUSER_BUILD_WITH_ASAN:BOOL=ON in CMakeLists.txt to OFF.

Proposed solution

Let setup.py always specify -DNVFUSER_BUILD_WITH_ASAN.

[wujingyue]

!test

[github-actions]

Description

• Always specify -DNVFUSER_BUILD_WITH_ASAN in CMake configuration

• Introduce helper function on_or_off for boolean flags

• Remove redundant ASAN check in configuration

---

Changes walkthrough 📝

	Relevant files
Enhancement	utils.py Always specify ASAN flag in CMake                                                python/utils.py Added on_or_off function to convert boolean to "ON"/"OFF" Modified cmake function to always include -DNVFUSER_BUILD_WITH_ASAN Removed redundant ASAN check in configuration +4/-2

---

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 No relevant tests

⚡ Recommended focus areas for review Possible Issue The proposed solution assumes that config.build_with_asan is always set, which might not be the case. Ensure that config.build_with_asan is defined and defaults to False if not provided. def on_or_off(flag: bool) -> str: return "ON" if flag else "OFF" Redundancy The old code conditionally appends -DNVFUSER_BUILD_WITH_ASAN=ON if config.build_with_asan is True. The new code always appends -DNVFUSER_BUILD_WITH_ASAN with either ON or OFF. Ensure that this change does not introduce unnecessary complexity or potential issues. f"-DNVFUSER_BUILD_WITH_ASAN={on_or_off(config.build_with_asan)}",

[rdspring1]

!test --dev

[xwang233]

The CI script of dev branch (--dev) has already been merged and had no difference from the default CI trigger.

I'll take a look at the PR later.

[xwang233]

This change seems a bit weird to me. I want to ask, if we add NVFUSER_BUILD_WITH_ASAN to cmake definitions, should we also add all other build configs?

When you build nvfuser the first time with python setup.py develop, the cmake build cache is already populated. Without cleaning up the build, the second python setup.py develop is expected to be an incremental build that follows similar build configs as your first build. If you don't want ASAN in the second build, perhaps you should clean up the build and uninstall first?

If the option for NVFUSER_BUILD_WITH_ASAN is turned off in CMake here

Fuser/CMakeLists.txt

Lines 374 to 379 in 07effe8

	if(NVFUSER_BUILD_WITH_ASAN)
	target_compile_options(codegen_internal PRIVATE -fsanitize=address)
	target_link_options(codegen_internal PUBLIC -fsanitize=address)
	target_link_options(nvfuser_codegen PUBLIC -fsanitize=address)
	endif()

, basically all objects of the codegen_internal target should be recompiled.

[wujingyue]

should we also add all other build configs?

I think so. I'm happy to do that if you think this PR is moving to the right direction.

the second python setup.py develop is expected to be an incremental build

That's exactly the part I wanted to check with you -- user-facing behavior.

I wrote down my motivation in the PR description in case you missed it. To me, there's an inconsistency between switching from noasan to asan and switching from asan to noasan. The former rebuilds but the latter doesn't. This user-facing behavior sounds counter-intuitive to me; if both had rebuilt, I would have accepted the behavior.

Does the problem now make sense to you, and do you think it needs a fix?

[xwang233]

To me, there's an inconsistency between switching from noasan to asan and switching from asan to noasan. The former rebuilds but the latter doesn't.

I agree, that sounds counter-intuitive.

I'd take a look and see if there are standards or conventions for pip editable install. Either way won't change our release behavior since we only build once in CI. Also, let @jjsjann123 take a look.

[jjsjann123]

I'm on board with the direction. i.e. let's make options explicit.