Skip to content

Conversation

@jurahul
Copy link
Contributor

@jurahul jurahul commented Apr 15, 2025

  • Use StringRef type for Modifier instead of const char *.
  • Remove Modifier arg from functions that do not need them.

- Use StringRef type for Modifier instead of const char *.
- Remove Modifier arg from functions that do not need them.
@jurahul jurahul changed the title [NFC][NVPTX] Use StringRef for Modifier arg. [NFC][NVPTX] Use StringRef for Modifier arg in NVPTXInstPrinter Apr 15, 2025
@jurahul jurahul marked this pull request as ready for review April 15, 2025 16:21
@jurahul jurahul requested a review from AlexMaclean April 15, 2025 16:21
@llvmbot
Copy link
Member

llvmbot commented Apr 15, 2025

@llvm/pr-subscribers-backend-nvptx

Author: Rahul Joshi (jurahul)

Changes
  • Use StringRef type for Modifier instead of const char *.
  • Remove Modifier arg from functions that do not need them.

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

2 Files Affected:

  • (modified) llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXInstPrinter.cpp (+11-17)
  • (modified) llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXInstPrinter.h (+12-17)
diff --git a/llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXInstPrinter.cpp b/llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXInstPrinter.cpp
index e42e738b9973f..4e2e4c99df803 100644
--- a/llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXInstPrinter.cpp
+++ b/llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXInstPrinter.cpp
@@ -95,10 +95,9 @@ void NVPTXInstPrinter::printOperand(const MCInst *MI, unsigned OpNo,
 }
 
 void NVPTXInstPrinter::printCvtMode(const MCInst *MI, int OpNum, raw_ostream &O,
-                                    const char *M) {
+                                    StringRef Modifier) {
   const MCOperand &MO = MI->getOperand(OpNum);
   int64_t Imm = MO.getImm();
-  llvm::StringRef Modifier(M);
 
   if (Modifier == "ftz") {
     // FTZ flag
@@ -155,10 +154,9 @@ void NVPTXInstPrinter::printCvtMode(const MCInst *MI, int OpNum, raw_ostream &O,
 }
 
 void NVPTXInstPrinter::printCmpMode(const MCInst *MI, int OpNum, raw_ostream &O,
-                                    const char *M) {
+                                    StringRef Modifier) {
   const MCOperand &MO = MI->getOperand(OpNum);
   int64_t Imm = MO.getImm();
-  llvm::StringRef Modifier(M);
 
   if (Modifier == "ftz") {
     // FTZ flag
@@ -229,8 +227,7 @@ void NVPTXInstPrinter::printCmpMode(const MCInst *MI, int OpNum, raw_ostream &O,
 }
 
 void NVPTXInstPrinter::printLdStCode(const MCInst *MI, int OpNum,
-                                     raw_ostream &O, const char *M) {
-  llvm::StringRef Modifier(M);
+                                     raw_ostream &O, StringRef Modifier) {
   const MCOperand &MO = MI->getOperand(OpNum);
   int Imm = (int)MO.getImm();
   if (Modifier == "sem") {
@@ -329,10 +326,9 @@ void NVPTXInstPrinter::printLdStCode(const MCInst *MI, int OpNum,
 }
 
 void NVPTXInstPrinter::printMmaCode(const MCInst *MI, int OpNum, raw_ostream &O,
-                                    const char *M) {
+                                    StringRef Modifier) {
   const MCOperand &MO = MI->getOperand(OpNum);
   int Imm = (int)MO.getImm();
-  llvm::StringRef Modifier(M);
   if (Modifier.empty() || Modifier == "version") {
     O << Imm; // Just print out PTX version
     return;
@@ -346,9 +342,8 @@ void NVPTXInstPrinter::printMmaCode(const MCInst *MI, int OpNum, raw_ostream &O,
 }
 
 void NVPTXInstPrinter::printMemOperand(const MCInst *MI, int OpNum,
-                                       raw_ostream &O, const char *M) {
+                                       raw_ostream &O, StringRef Modifier) {
   printOperand(MI, OpNum, O);
-  llvm::StringRef Modifier(M);
 
   if (Modifier == "add") {
     O << ", ";
@@ -363,7 +358,7 @@ void NVPTXInstPrinter::printMemOperand(const MCInst *MI, int OpNum,
 }
 
 void NVPTXInstPrinter::printOffseti32imm(const MCInst *MI, int OpNum,
-                                         raw_ostream &O, const char *Modifier) {
+                                         raw_ostream &O) {
   auto &Op = MI->getOperand(OpNum);
   assert(Op.isImm() && "Invalid operand");
   if (Op.getImm() != 0) {
@@ -373,13 +368,13 @@ void NVPTXInstPrinter::printOffseti32imm(const MCInst *MI, int OpNum,
 }
 
 void NVPTXInstPrinter::printHexu32imm(const MCInst *MI, int OpNum,
-                                      raw_ostream &O, const char *Modifier) {
+                                      raw_ostream &O) {
   int64_t Imm = MI->getOperand(OpNum).getImm();
   O << formatHex(Imm) << "U";
 }
 
 void NVPTXInstPrinter::printProtoIdent(const MCInst *MI, int OpNum,
-                                       raw_ostream &O, const char *Modifier) {
+                                       raw_ostream &O) {
   const MCOperand &Op = MI->getOperand(OpNum);
   assert(Op.isExpr() && "Call prototype is not an MCExpr?");
   const MCExpr *Expr = Op.getExpr();
@@ -388,7 +383,7 @@ void NVPTXInstPrinter::printProtoIdent(const MCInst *MI, int OpNum,
 }
 
 void NVPTXInstPrinter::printPrmtMode(const MCInst *MI, int OpNum,
-                                     raw_ostream &O, const char *Modifier) {
+                                     raw_ostream &O) {
   const MCOperand &MO = MI->getOperand(OpNum);
   int64_t Imm = MO.getImm();
 
@@ -419,10 +414,9 @@ void NVPTXInstPrinter::printPrmtMode(const MCInst *MI, int OpNum,
 }
 
 void NVPTXInstPrinter::printTmaReductionMode(const MCInst *MI, int OpNum,
-                                             raw_ostream &O,
-                                             const char *Modifier) {
+                                             raw_ostream &O) {
   const MCOperand &MO = MI->getOperand(OpNum);
-  using RedTy = llvm::nvvm::TMAReductionOp;
+  using RedTy = nvvm::TMAReductionOp;
 
   switch (static_cast<RedTy>(MO.getImm())) {
   case RedTy::ADD:
diff --git a/llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXInstPrinter.h b/llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXInstPrinter.h
index 2b19386ef17fe..a2dd772cd86d0 100644
--- a/llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXInstPrinter.h
+++ b/llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXInstPrinter.h
@@ -37,25 +37,20 @@ class NVPTXInstPrinter : public MCInstPrinter {
 
   void printOperand(const MCInst *MI, unsigned OpNo, raw_ostream &O);
   void printCvtMode(const MCInst *MI, int OpNum, raw_ostream &O,
-                    const char *Modifier = nullptr);
+                    StringRef Modifier = {});
   void printCmpMode(const MCInst *MI, int OpNum, raw_ostream &O,
-                    const char *Modifier = nullptr);
-  void printLdStCode(const MCInst *MI, int OpNum,
-                     raw_ostream &O, const char *Modifier = nullptr);
+                    StringRef Modifier = {});
+  void printLdStCode(const MCInst *MI, int OpNum, raw_ostream &O,
+                     StringRef Modifier = {});
   void printMmaCode(const MCInst *MI, int OpNum, raw_ostream &O,
-                    const char *Modifier = nullptr);
-  void printMemOperand(const MCInst *MI, int OpNum,
-                       raw_ostream &O, const char *Modifier = nullptr);
-  void printOffseti32imm(const MCInst *MI, int OpNum, raw_ostream &O,
-                         const char *Modifier = nullptr);
-  void printHexu32imm(const MCInst *MI, int OpNum, raw_ostream &O,
-                      const char *Modifier = nullptr);
-  void printProtoIdent(const MCInst *MI, int OpNum,
-                       raw_ostream &O, const char *Modifier = nullptr);
-  void printPrmtMode(const MCInst *MI, int OpNum, raw_ostream &O,
-                     const char *Modifier = nullptr);
-  void printTmaReductionMode(const MCInst *MI, int OpNum, raw_ostream &O,
-                             const char *Modifier = nullptr);
+                    StringRef Modifier = {});
+  void printMemOperand(const MCInst *MI, int OpNum, raw_ostream &O,
+                       StringRef Modifier = {});
+  void printOffseti32imm(const MCInst *MI, int OpNum, raw_ostream &O);
+  void printHexu32imm(const MCInst *MI, int OpNum, raw_ostream &O);
+  void printProtoIdent(const MCInst *MI, int OpNum, raw_ostream &O);
+  void printPrmtMode(const MCInst *MI, int OpNum, raw_ostream &O);
+  void printTmaReductionMode(const MCInst *MI, int OpNum, raw_ostream &O);
 };
 
 }

Copy link
Member

@AlexMaclean AlexMaclean left a comment

Choose a reason for hiding this comment

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

LGTM

@jurahul jurahul merged commit 1545f11 into llvm:main Apr 15, 2025
15 checks passed
@jurahul jurahul deleted the nvptx_inst_printer_stringref branch April 15, 2025 17:39
@llvm-ci
Copy link
Collaborator

llvm-ci commented Apr 15, 2025

LLVM Buildbot has detected a new failure on builder lldb-arm-ubuntu running on linaro-lldb-arm-ubuntu while building llvm at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/18/builds/14531

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
PASS: lldb-api :: tools/lldb-dap/instruction-breakpoint/TestDAP_instruction_breakpoint.py (1170 of 2954)
PASS: lldb-api :: tools/lldb-dap/disconnect/TestDAP_disconnect.py (1171 of 2954)
PASS: lldb-api :: tools/lldb-dap/io/TestDAP_io.py (1172 of 2954)
PASS: lldb-api :: tools/lldb-dap/locations/TestDAP_locations.py (1173 of 2954)
PASS: lldb-api :: tools/lldb-dap/memory/TestDAP_memory.py (1174 of 2954)
PASS: lldb-api :: tools/lldb-dap/optimized/TestDAP_optimized.py (1175 of 2954)
PASS: lldb-api :: tools/lldb-dap/output/TestDAP_output.py (1176 of 2954)
PASS: lldb-api :: tools/lldb-dap/launch/TestDAP_launch.py (1177 of 2954)
PASS: lldb-api :: tools/lldb-dap/repl-mode/TestDAP_repl_mode_detection.py (1178 of 2954)
UNRESOLVED: lldb-api :: tools/lldb-dap/evaluate/TestDAP_evaluate.py (1179 of 2954)
******************** TEST 'lldb-api :: tools/lldb-dap/evaluate/TestDAP_evaluate.py' FAILED ********************
Script:
--
/usr/bin/python3.10 /home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./lib --env LLVM_INCLUDE_DIR=/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/include --env LLVM_TOOLS_DIR=/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin --arch armv8l --build-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex --lldb-module-cache-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin/lldb --compiler /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin/clang --dsymutil /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./bin --lldb-obj-root /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/tools/lldb --lldb-libs-dir /home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/./lib /home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/tools/lldb-dap/evaluate -p TestDAP_evaluate.py
--
Exit Code: 1

Command Output (stdout):
--
lldb version 21.0.0git (https://github.com/llvm/llvm-project.git revision 1545f1139127b92be15fcd2964114028a2d07194)
  clang revision 1545f1139127b92be15fcd2964114028a2d07194
  llvm revision 1545f1139127b92be15fcd2964114028a2d07194
Skipping the following test categories: ['libc++', 'dsym', 'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
========= DEBUG ADAPTER PROTOCOL LOGS =========
1744739760.846108198 --> (stdin/stdout) {"command":"initialize","type":"request","arguments":{"adapterID":"lldb-native","clientID":"vscode","columnsStartAt1":true,"linesStartAt1":true,"locale":"en-us","pathFormat":"path","supportsRunInTerminalRequest":true,"supportsVariablePaging":true,"supportsVariableType":true,"supportsStartDebuggingRequest":true,"supportsProgressReporting":true,"$__lldb_sourceInitFile":false},"seq":1}
1744739760.850243807 <-- (stdin/stdout) {"body":{"$__lldb_version":"lldb version 21.0.0git (https://github.com/llvm/llvm-project.git revision 1545f1139127b92be15fcd2964114028a2d07194)\n  clang revision 1545f1139127b92be15fcd2964114028a2d07194\n  llvm revision 1545f1139127b92be15fcd2964114028a2d07194","completionTriggerCharacters":["."," ","\t"],"exceptionBreakpointFilters":[{"default":false,"filter":"cpp_catch","label":"C++ Catch"},{"default":false,"filter":"cpp_throw","label":"C++ Throw"},{"default":false,"filter":"objc_catch","label":"Objective-C Catch"},{"default":false,"filter":"objc_throw","label":"Objective-C Throw"}],"supportTerminateDebuggee":true,"supportsBreakpointLocationsRequest":true,"supportsCancelRequest":true,"supportsCompletionsRequest":true,"supportsConditionalBreakpoints":true,"supportsConfigurationDoneRequest":true,"supportsDataBreakpoints":true,"supportsDelayedStackTraceLoading":true,"supportsDisassembleRequest":true,"supportsEvaluateForHovers":true,"supportsExceptionInfoRequest":true,"supportsExceptionOptions":true,"supportsFunctionBreakpoints":true,"supportsHitConditionalBreakpoints":true,"supportsInstructionBreakpoints":true,"supportsLogPoints":true,"supportsModulesRequest":true,"supportsReadMemoryRequest":true,"supportsRestartRequest":true,"supportsSetVariable":true,"supportsStepInTargetsRequest":true,"supportsSteppingGranularity":true,"supportsValueFormattingOptions":true},"command":"initialize","request_seq":1,"seq":0,"success":true,"type":"response"}
1744739760.851084948 --> (stdin/stdout) {"command":"launch","type":"request","arguments":{"program":"/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex/tools/lldb-dap/evaluate/TestDAP_evaluate.test_generic_evaluate_expressions/a.out","initCommands":["settings clear -all","settings set symbols.enable-external-lookup false","settings set target.inherit-tcc true","settings set target.disable-aslr false","settings set target.detach-on-error false","settings set target.auto-apply-fixits false","settings set plugin.process.gdb-remote.packet-timeout 60","settings set symbols.clang-modules-cache-path \"/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api\"","settings set use-color false","settings set show-statusline false"],"disableASLR":false,"enableAutoVariableSummaries":false,"enableSyntheticChildDebugging":false,"displayExtendedBacktrace":false,"commandEscapePrefix":null},"seq":2}
1744739760.851614475 <-- (stdin/stdout) {"body":{"category":"console","output":"Running initCommands:\n"},"event":"output","seq":0,"type":"event"}
1744739760.851679087 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings clear -all\n"},"event":"output","seq":0,"type":"event"}
1744739760.851695776 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings set symbols.enable-external-lookup false\n"},"event":"output","seq":0,"type":"event"}
1744739760.851708651 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings set target.inherit-tcc true\n"},"event":"output","seq":0,"type":"event"}
1744739760.851720810 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings set target.disable-aslr false\n"},"event":"output","seq":0,"type":"event"}
1744739760.851732731 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings set target.detach-on-error false\n"},"event":"output","seq":0,"type":"event"}
1744739760.851744175 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings set target.auto-apply-fixits false\n"},"event":"output","seq":0,"type":"event"}
1744739760.851757050 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings set plugin.process.gdb-remote.packet-timeout 60\n"},"event":"output","seq":0,"type":"event"}
1744739760.851798534 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings set symbols.clang-modules-cache-path \"/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex/module-cache-lldb/lldb-api\"\n"},"event":"output","seq":0,"type":"event"}
1744739760.851811409 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings set use-color false\n"},"event":"output","seq":0,"type":"event"}
1744739760.851823330 <-- (stdin/stdout) {"body":{"category":"console","output":"(lldb) settings set show-statusline false\n"},"event":"output","seq":0,"type":"event"}
1744739761.023812771 <-- (stdin/stdout) {"command":"launch","request_seq":2,"seq":0,"success":true,"type":"response"}
1744739761.023997784 <-- (stdin/stdout) {"body":{"isLocalProcess":true,"name":"/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/lldb-test-build.noindex/tools/lldb-dap/evaluate/TestDAP_evaluate.test_generic_evaluate_expressions/a.out","startMethod":"launch","systemProcessId":3277226},"event":"process","seq":0,"type":"event"}
1744739761.024028301 <-- (stdin/stdout) {"event":"initialized","seq":0,"type":"event"}
1744739761.025490761 --> (stdin/stdout) {"command":"setBreakpoints","type":"request","arguments":{"source":{"name":"main.cpp","path":"main.cpp"},"sourceModified":false,"lines":[25,29,12,37,42,46,47,50],"breakpoints":[{"line":25},{"line":29},{"line":12},{"line":37},{"line":42},{"line":46},{"line":47},{"line":50}]},"seq":3}
1744739761.038406610 <-- (stdin/stdout) {"body":{"breakpoint":{"column":14,"id":1,"instructionReference":"0x801110","line":25,"verified":true},"reason":"changed"},"event":"breakpoint","seq":0,"type":"event"}
1744739761.039095402 <-- (stdin/stdout) {"body":{"breakpoint":{"column":16,"id":2,"instructionReference":"0x80112C","line":29,"verified":true},"reason":"changed"},"event":"breakpoint","seq":0,"type":"event"}
1744739761.039983988 <-- (stdin/stdout) {"body":{"breakpoint":{"column":10,"id":3,"instructionReference":"0x8010AC","line":12,"verified":true},"reason":"changed"},"event":"breakpoint","seq":0,"type":"event"}

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.

4 participants