Skip to content

Conversation

@tbensonatl
Copy link
Collaborator

When MATX_EN_EXTENDED_LAMBDA or MATX_EN_CUDA_LINEINFO are enabled, propagate the --extended-lambda and -lineinfo nvcc flags to the matx interface target. Any application linking to the matx library will then also include these flags.

When MATX_EN_EXTENDED_LAMBDA or MATX_EN_CUDA_LINEINFO are enabled, propagate
the --extended-lambda and -lineinfo nvcc flags to the matx interface target.
Any application linking to the matx library will then also include these flags.

Signed-off-by: Thomas Benson <tbenson@nvidia.com>
@copy-pr-bot
Copy link

copy-pr-bot bot commented Nov 6, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@tbensonatl
Copy link
Collaborator Author

/build

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Greptile Summary

Changed how -lineinfo and --extended-lambda CUDA compiler flags are propagated from internal CMake variables to the matx interface target, ensuring external consumers automatically receive these flags when linking against MatX.

Key changes:

  • Replaced set(MATX_CUDA_FLAGS ...) with target_compile_options(matx INTERFACE ...) for both flags
  • Used generator expressions $<$<COMPILE_LANGUAGE:CUDA>:...> to apply flags only to CUDA compilation
  • Added explanatory comment about propagating flags to consumers

Minor concern:

  • Line 150 still includes --extended-lambda in MATX_CUDA_FLAGS for internal builds, which may result in duplicate flags when combined with the new interface target setting at line 161 (when MATX_EN_EXTENDED_LAMBDA is ON, which is the default)

Confidence Score: 4/5

  • Safe to merge with minor optimization opportunity regarding duplicate flags
  • The change correctly implements the intended functionality of propagating compiler flags to external consumers. The use of target_compile_options with INTERFACE scope and generator expressions is the proper CMake approach. The only issue is a potential duplicate --extended-lambda flag for internal builds, which is non-critical (compilers typically ignore duplicate flags)
  • No files require special attention - the change is straightforward and well-scoped

Important Files Changed

File Analysis

Filename Score Overview
CMakeLists.txt 4/5 Correctly propagates -lineinfo and --extended-lambda flags to consumers via interface target; potential duplicate flags for internal builds (line 150 vs 161)

Sequence Diagram

sequenceDiagram
    participant User as External User/App
    participant CMake as CMake Configuration
    participant MatX as matx Interface Target
    participant Consumer as Consumer Application

    Note over CMake,MatX: PR Changes: Lines 154-162

    CMake->>CMake: Check MATX_EN_CUDA_LINEINFO or Debug build
    alt MATX_EN_CUDA_LINEINFO enabled OR Debug build
        CMake->>MatX: target_compile_options(matx INTERFACE -lineinfo)
        Note over MatX: Flag stored in interface target
    end

    CMake->>CMake: Check MATX_EN_EXTENDED_LAMBDA (default: ON)
    alt MATX_EN_EXTENDED_LAMBDA enabled
        CMake->>MatX: target_compile_options(matx INTERFACE --extended-lambda)
        Note over MatX: Flag stored in interface target
    end

    User->>Consumer: Link against matx::matx
    MatX->>Consumer: Propagate -lineinfo (if enabled)
    MatX->>Consumer: Propagate --extended-lambda (if enabled)
    Note over Consumer: CUDA compiler receives flags automatically
Loading

Additional Comments (1)

  1. CMakeLists.txt, line 150 (link)

    style: potential duplicate: --extended-lambda appears here and is also conditionally added to the interface target at line 161 when MATX_EN_EXTENDED_LAMBDA is enabled (which defaults to ON). Internal builds that link to matx::matx may receive the flag twice.

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@cliffburdick
Copy link
Collaborator

/build

@cliffburdick cliffburdick merged commit ac48747 into main Nov 7, 2025
1 check passed
@cliffburdick cliffburdick deleted the add-lineinfo-flag-to-interface-target branch November 7, 2025 02:52
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.

3 participants