Use ttgir line number instead of source line number to help matching from ttgir to llir/ptx #3458

manman-ren · 2024-03-25T16:55:46Z

This can help us analyzing the mapping from ttgir to llir/ptx. When used with performance tools, it can provide a breakdown on ttgir instructions.
Added one env variable:
"USE_TTGIR_LOC": will not emit location information in the dumped ttgir, will re-parse the ttgir file and use ttgir line numbers as debug info
When running on vector-add with USE_TTGIR_LOC=1:
ttgir: #loc = loc("/cache-path/add_kernel.ttgir":2:1)
llir: !3 = !DIFile(filename: "add_kernel.ttgir", directory: "/cache-path")

manman-ren · 2024-03-25T16:56:08Z

Open for discussions. CC @Jokeren @bertmaher

Jokeren · 2024-03-25T17:43:18Z

DISABLE_LOC_DUMP is fine to me, but it might be better if DISABLE_LOC_DUMP refers to disabling all location information.
DISABLE_LOC_DUMP = 0, USE_TTGIR_LOC = 1, ttgir loc info.
DISABLE_LOC_DUMP = 1, USE_TTGIR_LOC = 1, no loc info.
DISABLE_LOC_DUMP = 0, USE_TTGIR_LOC = 0, python loc info.
DISABLE_LOC_DUMP = 1, USE_TTGIR_LOC = 0, no loc info.

Jokeren · 2024-03-25T17:49:48Z

python/triton/compiler/compiler.py

@@ -276,6 +276,12 @@ def compile(src, target=None, options=None):
            print(f"\nOverriding kernel with file {ir_filename}")
            full_name = fn_override_manager.get_file(ir_filename)
            next_module = parse(full_name, ext, context)
+        # use an env variable to parse ttgir from file
+        use_ttgir_loc = os.environ.get("USE_TTGIR_LOC", "0") == "1"


Make this line outside of the loop

Jokeren · 2024-03-25T17:55:01Z

python/triton/compiler/compiler.py

+        use_ttgir_loc = os.environ.get("USE_TTGIR_LOC", "0") == "1"
+        if use_ttgir_loc and ext == "ttgir":
+            ttgir_full_name = fn_cache_manager.get_file(ir_filename)
+            next_module = parse(ttgir_full_name, ext, context)


Why parse it again if next_module is already a ttgir module?

We need to parse it again to get the line numbers of the ttgir file :] When there is no #loc in the ttgir file, parsing it again will set the location of ops in the ttgir to the line number in the ttgir file.

I get it now. Thanks.

I think the following behavior might make more sense as I described above.

DISABLE_LOC_DUMP = 0, USE_TTGIR_LOC = 1, ttgir loc info. DISABLE_LOC_DUMP = 1, USE_TTGIR_LOC = 1, no loc info. DISABLE_LOC_DUMP = 0, USE_TTGIR_LOC = 0, python loc info. DISABLE_LOC_DUMP = 1, USE_TTGIR_LOC = 0, no loc info.

Bert is probably on PTO this week

I get it now. Thanks.

I think the following behavior might make more sense as I described above.

DISABLE_LOC_DUMP = 0, USE_TTGIR_LOC = 1, ttgir loc info. DISABLE_LOC_DUMP = 1, USE_TTGIR_LOC = 1, no loc info. DISABLE_LOC_DUMP = 0, USE_TTGIR_LOC = 0, python loc info. DISABLE_LOC_DUMP = 1, USE_TTGIR_LOC = 0, no loc info.

Yeah this makes sense. But we also have TRITON_DISABLE_LINE_INFO which decides whether we will have locinfo attached. DISABLE_LOC_DUMP is for whether or not to dump the locinfo when locinfo exists.

The question is when DISABLE_LOC_DUMP = 1 and TRITON_DISABLE_LINE_INFO = 0, should we have locinfo attached but not dumped?

Does it make sense to only use TRITON_DISABLE_LINE_INFO but not DISABLE_LOC_DUMP? I'm a bit worried about the growth of env flags.

Does it make sense to only use TRITON_DISABLE_LINE_INFO but not DISABLE_LOC_DUMP? I'm a bit worried about the growth of env flags.

If we set TRITON_DISABLE_LINE_INFO to 1, there will be no debug info/location info. I wonder if it makes sense to get rid of DISABLE_LOC_DUMP and only use USE_TTGIR_LOC, when USE_TTGIR_LOC is true, we will not dump location info in the cached ttgir so when we parse it again, it will use the ttgir line number.

Sure, that seems better to me

manman-ren · 2024-04-05T16:28:22Z

@Jokeren Can you take a look at this diff again? Thanks!

Jokeren

Just one last comment. And maybe you want to refactor the commit message.

Jokeren · 2024-04-05T17:37:36Z

python/src/ir.cc

@@ -491,7 +495,8 @@ void init_triton_ir(py::module &&m) {
          throw std::runtime_error("Parse MLIR file failed.");
        // locations are incompatible with ptx < 7.5 !
        module->walk([](Operation *op) {
-          op->setLoc(UnknownLoc::get(op->getContext()));
+          if (!::triton::tools::getBoolEnv("USE_TTGIR_LOC"))


Move it outside of walk

manman-ren · 2024-04-09T17:27:02Z

@Jokeren Can you take another look? Thanks!

jlebar · 2024-04-09T19:15:31Z

Can you add this to the README in the new environment variables section?

manman-ren · 2024-04-09T21:51:29Z

Can you add this to the README in the new environment variables section?

Yes! I will. Saw your additions to the README for debugging env variables.

This can help use analyzing the mapping from ttgir to llir/ptx. When used with performance tools, it can provide a breakdown on ttgir instructions. Added two env variables: "DISABLE_LOC_DUMP": the cached IR files will not have location info "USE_TTGIR_LOC": we will re-parse the ttgir file and use ttgir line numbers When running on vector-add: ttgir: #loc = loc("/cache-path/add_kernel.ttgir":2:1) llir: !3 = !DIFile(filename: "add_kernel.ttgir", directory: "/cache-path")

manman-ren requested a review from ptillet as a code owner March 25, 2024 16:55

Jokeren reviewed Mar 25, 2024

View reviewed changes

manman-ren force-pushed the use-ttgir-line-number branch from 8f8e7a0 to f2342fe Compare April 3, 2024 22:34

Jokeren reviewed Apr 5, 2024

View reviewed changes

manman-ren force-pushed the use-ttgir-line-number branch from f2342fe to 8558685 Compare April 5, 2024 20:47

Jokeren approved these changes Apr 9, 2024

View reviewed changes

mren2 added 5 commits April 9, 2024 17:16

fix clang-format

cb32e41

address review comments

08cb358

address review comments

1c44794

update README

93a91f5

manman-ren force-pushed the use-ttgir-line-number branch from 8558685 to 93a91f5 Compare April 10, 2024 00:20

jlebar approved these changes Apr 10, 2024

View reviewed changes

jlebar enabled auto-merge (squash) April 10, 2024 00:22

jlebar merged commit efbf37f into triton-lang:main Apr 10, 2024
5 checks passed

manman-ren deleted the use-ttgir-line-number branch May 2, 2024 15:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use ttgir line number instead of source line number to help matching from ttgir to llir/ptx #3458

Use ttgir line number instead of source line number to help matching from ttgir to llir/ptx #3458

manman-ren commented Mar 25, 2024 •

edited

Loading

manman-ren commented Mar 25, 2024

Jokeren commented Mar 25, 2024 •

edited

Loading

Jokeren Mar 25, 2024

Jokeren Mar 25, 2024

manman-ren Mar 25, 2024

Jokeren Mar 26, 2024

Jokeren Mar 26, 2024

manman-ren Apr 1, 2024

Jokeren Apr 1, 2024

manman-ren Apr 1, 2024

Jokeren Apr 1, 2024

manman-ren commented Apr 5, 2024

Jokeren left a comment

Jokeren Apr 5, 2024

manman-ren commented Apr 9, 2024

jlebar commented Apr 9, 2024

manman-ren commented Apr 9, 2024

Use ttgir line number instead of source line number to help matching from ttgir to llir/ptx #3458

Use ttgir line number instead of source line number to help matching from ttgir to llir/ptx #3458

Conversation

manman-ren commented Mar 25, 2024 • edited Loading

manman-ren commented Mar 25, 2024

Jokeren commented Mar 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manman-ren commented Apr 5, 2024

Jokeren left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manman-ren commented Apr 9, 2024

jlebar commented Apr 9, 2024

manman-ren commented Apr 9, 2024

manman-ren commented Mar 25, 2024 •

edited

Loading

Jokeren commented Mar 25, 2024 •

edited

Loading