[XPU][TritonIntelGPUToLLVM] Do not generate duplicated string constants #3137
Conversation
Do not generate duplicated string constants as result of `tt.assert` or `tt.print` operations. Use a string constant cache to avoid duplication. Signed-off-by: victor-eds <victor.perez@codeplay.com>
|
Fixing this issue would result in overhead in the base case (no Alternatively, we can merge this PR. |
Hi @victor-eds, is the overhead significant? Maybe it is acceptable, considering that we seem to be saving memory? |
I don't think the overhead is significant, but I wonder if we want to go with any additional overhead to save memory on "debug" scenarios (asserts don't have effect unless
|
| UnknownLoc::get(rewriter.getContext()), rewriter, "printfFormat_", | ||
| msgNewline, TritonGEN::TritonGENMemorySpace::kUniformConstant); | ||
| Value msgValue = | ||
| static_cast<const intel::TargetInfo &>(targetInfo) |
There was a problem hiding this comment.
Can you change the member to this type, or does it have to remain a TargetInfoBase?
| do { | ||
| stringConstName.clear(); | ||
| (name + Twine(stringNumber++)).toStringRef(stringConstName); | ||
| } while (moduleOp.lookupSymbol(stringConstName)); |
There was a problem hiding this comment.
Does it matter to have the symbols nicely numbered per name? A simpler way would be just taking globals.size().
|
Which overhead(s) are we talking about here? I might be missing something, but I don't see how kernels not using printf or assert would be affected. And also there doesn't seem to be a runtime overhead at all. In that case, I'd be in favour of landing this PR. |
Do not generate duplicated string constants as result of
tt.assertortt.printoperations. Use a string constant cache to avoid duplication.Closes #3054.