-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[XPU][OptRed] Define triton_intel_gpu.simd_reduce
and use in optimized transposed reduction
#2907
base: main
Are you sure you want to change the base?
Conversation
5e578e3
to
114add9
Compare
Something is off with the vISA generated. I'm looking into that. Delay reviews. |
IGC vISA parser fails to parse this when combined with DPAS instruction. Waiting for fix. |
@@ -0,0 +1,24 @@ | |||
// RUN: triton-opt %s -split-input-file --intel-allocate-shared-memory --convert-triton-intel-gpu-to-llvm | FileCheck %s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit]: -split-input-file isn't required
// To get the asm code: | ||
// builder.dump() | ||
// ``` | ||
// To get all the `mlir::Value` used in the VISA code, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// To get all the `mlir::Value` used in the VISA code, | |
// To get all the `mlir::Value` used in the VISA code: |
Operand *newListOperand() { return newOperand(); } | ||
|
||
Operand *newListOperand(ArrayRef<std::pair<mlir::Value, std::string>> items) { | ||
auto *list = newOperand(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto *list = newOperand(); | |
Operand *list = newOperand(); |
for (auto &item : items) { | ||
list->listAppend(newOperand(item.first, item.second)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for (auto &item : items) { | |
list->listAppend(newOperand(item.first, item.second)); | |
for (auto &[val, constraint] : items) { | |
list->listAppend(newOperand(val, constraint)); |
} | ||
|
||
Operand *listGet(size_t nth) const { | ||
assert(nth < list.size()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add assert msg here
[&](std::unique_ptr<Operand> &a, std::unique_ptr<Operand> &b) { | ||
auto ida = std::find(order.begin(), order.end(), a.get()); | ||
auto idb = std::find(order.begin(), order.end(), b.get()); | ||
assert(ida != order.end()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
assert msgs missing
#include "mlir/Transforms/DialectConversion.h" | ||
#include "triton/Conversion/TritonGPUToLLVM/AsmFormat.h" | ||
#include "llvm/Support/raw_ostream.h" | ||
// TODO(Superjomn): unify to llvm::raw_string_ostream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove
|
||
VISABuilder::Operand *VISABuilder::newOperand(StringRef constraint, bool init) { | ||
// Constraint should be something like "=rw" | ||
assert(constraint[0] == '='); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add assert msg please
VISABuilder::Operand *VISABuilder::newOperand(StringRef constraint, bool init) { | ||
// Constraint should be something like "=rw" | ||
assert(constraint[0] == '='); | ||
auto *opr = newOperand(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto *opr = newOperand(); | |
Operand *opr = newOperand(); |
|
||
VISABuilder::Operand *VISABuilder::newOperand(unsigned operandIndex) { | ||
assert(operandIndex < oprCounter && "operand index out of range"); | ||
auto *opr = newOperand(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto *opr = newOperand(); | |
Operand *opr = newOperand(); |
1fc7a53
to
4457fd1
Compare
…zed transposed reduction Define SIMD transpose-reduce operation performing a SIMD reduction while transposing the implicit SIMD matrix. See description definition for further context. Using this operation in the transpose reduction pass allows us to perform the optimization while not using SLM. Signed-off-by: victor-eds <[email protected]> Co-authored-by: chengjunlu <[email protected]> Signed-off-by: Victor Perez <[email protected]>
4fc99fa
to
d0cbd12
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest to first create a PR to add VISABuilder
, with unit test like third_party/nvidia/unittest/Conversion/TritonGPUToLLVM/PTXAsmFormatTest.cpp
.
Can we also measure if there are any performance impact with this PR?
As discussed in our offline architecture mtg @chengjunlu will review this PR and determine whether this implementation is still needed or can be super seeded by the his approach. |
Define SIMD transpose-reduce operation performing a SIMD reduction while transposing the implicit SIMD matrix. See description definition for further context.
Using this operation in the transpose reduction pass allows us to perform the optimization while not using SLM.