Skip to content

Conversation

GeorgeWeb
Copy link

Passing -g to ptxas with any optimizations enabled is not allowed because, ptxas does not support optimized debugging.

@GeorgeWeb GeorgeWeb requested a review from a team as a code owner September 5, 2023 13:39
@github-actions github-actions bot added the clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' label Sep 5, 2023
…th optimizations

Passing -g to ptxas with any optimizations enabled is not allowed because,
ptxas does not support optimized debugging.
@GeorgeWeb GeorgeWeb force-pushed the georgi/ptxas-warning-diagnostic-no-debuginfo branch from 0efb5b6 to 757383f Compare September 5, 2023 13:43
@GeorgeWeb GeorgeWeb changed the title [NVPTX] Add a warning that device debug info does not work with optimizations [Driver][NVPTX] Add a warning that device debug info does not work with optimizations Sep 5, 2023
@GeorgeWeb
Copy link
Author

@Artem-B Hi, sorry for the direct ping. I was just wondering if it's okay to take a quick look at this patch.
(Didn't know how to ping folks subscribed to clang-driver).
Thanks! 🙏

@@ -413,13 +413,25 @@ void NVPTX::Assembler::ConstructJob(Compilation &C, const JobAction &JA,
// TODO: Perhaps we should map host -O2 to ptxas -O3. -O3 is ptxas's
// default, so it may correspond more closely to the spirit of clang -O2.

bool noOptimization = A->getOption().matches(options::OPT_O0);
// Emit a driver diagnostic as warning if any -O option different from -O0,
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit too aggressive, IMO. E.g. -g1 or -gmlt works just fine.
The warning should apply only to full debug info. You should probably move the code below to where we check DIKind and issue the warning only if DIKind == FullDebug.

Then there's a question of whether the warning is useful in general. E.g. in a large project where optimization and debug options are controlled globally, the users will all of a sudden start getting the warnings. The builds with -Werror will be broken and the users will have no easy way to deal with that.

// RUN: %clang -### --target=x86_64-linux-gnu -O0 -g -c %s 2>&1 \
// RUN: --offload-arch=sm_35 --cuda-path=%S/Inputs/CUDA/usr/local/cuda \
// RUN: | FileCheck -check-prefixes=CHECK,ARCH64,SM35,OPT0-DBG %s

// With debugging enabled, ptxas should be run with with no ptxas optimizations.
// RUN: %clang -### --target=x86_64-linux-gnu --cuda-noopt-device-debug -O2 -g -c %s 2>&1 \
Copy link
Member

Choose a reason for hiding this comment

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

We should have a test for the warning, so we're sure we're not spamming users with something they can't do much about.

@GeorgeWeb
Copy link
Author

Agreed not to go this path, hence closing this PR. Thanks for reviews.

@GeorgeWeb GeorgeWeb closed this Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl'
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants