Skip to content

Conversation

@justinfargnoli
Copy link
Contributor

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces f-string formatting with .format() method calls for string formatting in the lit configuration file. The change affects error messages and feature name generation in PTXAS-related functions.

  • Converts f-string literals to .format() method calls for consistency
  • Updates RuntimeError messages and ToolSubst parameters
  • Modifies feature name generation for PTXAS versions and architectures

@llvmbot
Copy link
Member

llvmbot commented Aug 28, 2025

@llvm/pr-subscribers-backend-nvptx

Author: Justin Fargnoli (justinfargnoli)

Changes

Fix #154439 (comment)


Full diff: https://github.com/llvm/llvm-project/pull/155912.diff

1 Files Affected:

  • (modified) llvm/test/lit.cfg.py (+6-6)
diff --git a/llvm/test/lit.cfg.py b/llvm/test/lit.cfg.py
index b23922d01483a..4a5d251f9a411 100644
--- a/llvm/test/lit.cfg.py
+++ b/llvm/test/lit.cfg.py
@@ -321,7 +321,7 @@ def ptxas_supported_isa_versions(ptxas, major_version, minor_version):
     if supported_isa_versions:
         return supported_isa_versions
     if major_version >= 13:
-        raise RuntimeError(f"ptxas {ptxas} does not support ISA version listing")
+        raise RuntimeError("ptxas {} does not support ISA version listing".format(ptxas))
 
     cuda_version_to_isa_version = {
         (12, 9): [(8, 8)],
@@ -404,7 +404,7 @@ def ptxas_supports_address_size_32(ptxas_executable):
         return False
     if "Missing .version directive at start of file" in result.stderr:
         return True
-    raise RuntimeError(f"Unexpected ptxas output: {result.stderr}")
+    raise RuntimeError("Unexpected ptxas output: {}".format(result.stderr))
 
 
 def enable_ptxas(ptxas_executable):
@@ -412,20 +412,20 @@ def enable_ptxas(ptxas_executable):
     tools.extend(
         [
             ToolSubst("%ptxas", ptxas_executable),
-            ToolSubst("%ptxas-verify", f"{ptxas_executable} -c -"),
+            ToolSubst("%ptxas-verify", "{} -c -".format(ptxas_executable)),
         ]
     )
 
     major_version, minor_version = ptxas_version(ptxas_executable)
-    config.available_features.add(f"ptxas-{major_version}.{minor_version}")
+    config.available_features.add("ptxas-{}.{}".format(major_version, minor_version))
 
     for major, minor in ptxas_supported_isa_versions(
         ptxas_executable, major_version, minor_version
     ):
-        config.available_features.add(f"ptxas-isa-{major}.{minor}")
+        config.available_features.add("ptxas-isa-{}.{}".format(major, minor))
 
     for sm in ptxas_supported_sms(ptxas_executable):
-        config.available_features.add(f"ptxas-sm_{sm}")
+        config.available_features.add("ptxas-sm_{}".format(sm))
 
     if ptxas_supports_address_size_32(ptxas_executable):
         config.available_features.add("ptxas-ptr32")

@justinfargnoli
Copy link
Contributor Author

@vvereschaka, is it possible for me to test out whether this fixes the issue?

I attempted to test #154439 before submitting it, but it appeared that my branch needed to be from "llvm-project" (as opposed to my fork of "llvm-project") to be accepted by BuildBot.

@justinfargnoli justinfargnoli changed the title [lit] Use .format over format strings [lit] Use .format() over format strings literals Aug 28, 2025
@github-actions
Copy link

github-actions bot commented Aug 28, 2025

✅ With the latest revision this PR passed the Python code formatter.

@vvereschaka
Copy link
Contributor

@vvereschaka, is it possible for me to test out whether this fixes the issue?

I attempted to test #154439 before submitting it, but it appeared that my branch needed to be from "llvm-project" (as opposed to my fork of "llvm-project") to be accepted by BuildBot.

@justinfargnoli not a problem, I'll test it on the buildbot locally

@vvereschaka
Copy link
Contributor

@justinfargnoli I still see the same error with these changes:

llvm-lit.py: C:\buildbot\temp\llvm-project\llvm\utils\lit\lit\llvm\config.py:57: note: using lit tools: C:\Program Files\Git\usr\bin
llvm-lit.py: C:\buildbot\temp\llvm-project\llvm\utils\lit\lit\TestingConfig.py:157: fatal: unable to parse config file 'C:\\buildbot\\temp\\llvm-project\\llvm\\test/lit.cfg.py', traceback: Traceback (most recent call last):
  File "C:\buildbot\temp\llvm-project\llvm\utils\lit\lit\TestingConfig.py", line 145, in load_from_path
    exec(compile(data, path, "exec"), cfg_globals, None)
  File "C:\buildbot\temp\llvm-project\llvm\test/lit.cfg.py", line 440, in <module>
    enable_ptxas(ptxas_executable)
  File "C:\buildbot\temp\llvm-project\llvm\test/lit.cfg.py", line 432, in enable_ptxas
    if ptxas_supports_address_size_32(ptxas_executable):
  File "C:\buildbot\temp\llvm-project\llvm\test/lit.cfg.py", line 409, in ptxas_supports_address_size_32
    raise RuntimeError("Unexpected ptxas output: {}".format(result.stderr))
RuntimeError: Unexpected ptxas output:

@justinfargnoli
Copy link
Contributor Author

Apologies, I misunderstood the error. I'll go forward with the revert.

@vvereschaka
Copy link
Contributor

vvereschaka commented Aug 28, 2025

@justinfargnoli , here is a trace output on Windows for that part of code:

[0/1] Running the LLVM regression testsllvm-lit.py: C:\buildbot\temp\llvm-project\llvm\utils\lit\lit\llvm\config.py:57: note: using lit tools: C:\Program Files\Git\usr\bin
+++ c:/buildbot/latest-cuda/bin/ptxas.exe -m 32
--- (stdout): ptxas fatal   : Value ' 32' is not defined for option 'machine'

--- (stderr):
llvm-lit.py: C:\buildbot\temp\llvm-project\llvm\utils\lit\lit\TestingConfig.py:157: fatal: unable to parse config file 'C:\\buildbot\\temp\\llvm-project\\llvm\\test/lit.cfg.py', traceback: Traceback (most recent call last):
  File "C:\buildbot\temp\llvm-project\llvm\utils\lit\lit\TestingConfig.py", line 145, in load_from_path
    exec(compile(data, path, "exec"), cfg_globals, None)
  File "C:\buildbot\temp\llvm-project\llvm\test/lit.cfg.py", line 444, in <module>
    enable_ptxas(ptxas_executable)
  File "C:\buildbot\temp\llvm-project\llvm\test/lit.cfg.py", line 436, in enable_ptxas
    if ptxas_supports_address_size_32(ptxas_executable):
  File "C:\buildbot\temp\llvm-project\llvm\test/lit.cfg.py", line 413, in ptxas_supports_address_size_32
    raise RuntimeError("Unexpected ptxas output: {}".format(result.stderr))
RuntimeError: Unexpected ptxas output:

the error message goes to stdout when using Win version of ptxas, but not to stderr. Looks like you need to check both outputs or merge them and check together at once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants