[mlir][LLVMIR] Add operand bundle support for llvm.intr.assume #112143
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Lancern
requested review from
gysit and
Dinistro
and removed request for
grypp
|
@llvm/pr-subscribers-mlir-sme @llvm/pr-subscribers-mlir Author: Sirui Mu (Lancern) ChangesThis PR adds operand bundle support for This PR actually contains two parts:
Patch is 38.43 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/112143.diff 17 Files Affected:
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td
index 448a171cf3e412..c56065af07b10e 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMIntrinsicOps.td
@@ -365,8 +365,8 @@ class LLVM_ConstrainedIntr<string mnem, int numArgs,
SmallVector<Value> mlirOperands;
SmallVector<NamedAttribute> mlirAttrs;
if (failed(moduleImport.convertIntrinsicArguments(
- llvmOperands.take_front( }] # numArgs # [{),
- {}, {}, mlirOperands, mlirAttrs))) {
+ llvmOperands.take_front( }] # numArgs # [{), {}, {}, {},
+ StringLiteral(""), StringLiteral(""), mlirOperands, mlirAttrs))) {
return failure();
}
@@ -426,7 +426,31 @@ def LLVM_USHLSat : LLVM_BinarySameArgsIntrOpI<"ushl.sat">;
//
def LLVM_AssumeOp
- : LLVM_ZeroResultIntrOp<"assume", []>, Arguments<(ins I1:$cond)>;
+ : LLVM_ZeroResultIntrOp<"assume", /*overloadedOperands=*/[], /*traits=*/[],
+ /*requiresAccessGroup=*/0,
+ /*requiresAliasAnalysis=*/0,
+ /*immArgPositions=*/[], /*immArgAttrNames=*/[],
+ /*opBundleSizesAttrName=*/"op_bundle_sizes",
+ /*opBundleTagsAttrName=*/"op_bundle_tags"> {
+ let arguments = (ins I1:$cond,
+ VariadicOfVariadic<LLVM_Type,
+ "op_bundle_sizes">:$op_bundle_operands,
+ DenseI32ArrayAttr:$op_bundle_sizes,
+ OptionalAttr<ArrayAttr>:$op_bundle_tags);
+
+ let assemblyFormat = [{
+ $cond
+ ( custom<OpBundles>($op_bundle_operands, type($op_bundle_operands),
+ $op_bundle_tags)^ )?
+ `:` `(` type($cond) `)` `->` `(` `)` attr-dict
+ }];
+
+ let builders = [
+ OpBuilder<(ins "Value":$cond)>
+ ];
+
+ let hasVerifier = 1;
+}
def LLVM_SSACopyOp : LLVM_OneResultIntrOp<"ssa.copy", [], [0],
[Pure, SameOperandsAndResultType]> {
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td
index c3d352d8d0dd48..dc59baed67472a 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOpBase.td
@@ -293,7 +293,9 @@ class LLVM_IntrOpBase<Dialect dialect, string opName, string enumName,
bit requiresAccessGroup = 0, bit requiresAliasAnalysis = 0,
bit requiresFastmath = 0,
list<int> immArgPositions = [],
- list<string> immArgAttrNames = []>
+ list<string> immArgAttrNames = [],
+ string opBundleSizesAttrName = "",
+ string opBundleTagsAttrName = "">
: LLVM_OpBase<dialect, opName, !listconcat(
!if(!gt(requiresAccessGroup, 0),
[DeclareOpInterfaceMethods<AccessGroupOpInterface>], []),
@@ -319,11 +321,14 @@ class LLVM_IntrOpBase<Dialect dialect, string opName, string enumName,
string immArgPositionsCpp = "{" # !interleave(immArgPositions, ", ") # "}";
string immArgAttrNamesCpp = "{" # !interleave(!foreach(name, immArgAttrNames,
"StringLiteral(\"" # name # "\")"), ", ") # "}";
+ string opBundleSizesAttrNameCpp = "StringLiteral(\"" # opBundleSizesAttrName # "\")";
+ string opBundleTagsAttrNameCpp = "StringLiteral(\"" # opBundleTagsAttrName # "\")";
string baseLlvmBuilder = [{
auto *inst = LLVM::detail::createIntrinsicCall(
builder, moduleTranslation, &opInst, llvm::Intrinsic::}] # !interleave([
enumName, "" # numResults, overloadedResultsCpp, overloadedOperandsCpp,
- immArgPositionsCpp, immArgAttrNamesCpp], ",") # [{);
+ immArgPositionsCpp, immArgAttrNamesCpp, opBundleSizesAttrNameCpp], ",")
+ # ", " # opBundleTagsAttrNameCpp # [{);
(void) inst;
}];
string baseLlvmBuilderCoda = !if(!gt(numResults, 0), "$res = inst;", "");
@@ -336,8 +341,11 @@ class LLVM_IntrOpBase<Dialect dialect, string opName, string enumName,
SmallVector<NamedAttribute> mlirAttrs;
if (failed(moduleImport.convertIntrinsicArguments(
llvmOperands,
+ opBundles,
}] # immArgPositionsCpp # [{,
}] # immArgAttrNamesCpp # [{,
+ }] # opBundleSizesAttrNameCpp # [{,
+ }] # opBundleTagsAttrNameCpp # [{,
mlirOperands,
mlirAttrs))
) {
@@ -382,11 +390,14 @@ class LLVM_IntrOp<string mnem, list<int> overloadedResults,
int numResults, bit requiresAccessGroup = 0,
bit requiresAliasAnalysis = 0, bit requiresFastmath = 0,
list<int> immArgPositions = [],
- list<string> immArgAttrNames = []>
+ list<string> immArgAttrNames = [],
+ string opBundleSizesAttrName = "",
+ string opBundleTagsAttrName = "">
: LLVM_IntrOpBase<LLVM_Dialect, "intr." # mnem, !subst(".", "_", mnem),
overloadedResults, overloadedOperands, traits,
numResults, requiresAccessGroup, requiresAliasAnalysis,
- requiresFastmath, immArgPositions, immArgAttrNames>;
+ requiresFastmath, immArgPositions, immArgAttrNames,
+ opBundleSizesAttrName, opBundleTagsAttrName>;
// Base class for LLVM intrinsic operations returning no results. Places the
// intrinsic into the LLVM dialect and prefixes its name with "intr.".
@@ -407,10 +418,13 @@ class LLVM_ZeroResultIntrOp<string mnem, list<int> overloadedOperands = [],
bit requiresAccessGroup = 0,
bit requiresAliasAnalysis = 0,
list<int> immArgPositions = [],
- list<string> immArgAttrNames = []>
+ list<string> immArgAttrNames = [],
+ string opBundleSizesAttrName = "",
+ string opBundleTagsAttrName = "">
: LLVM_IntrOp<mnem, [], overloadedOperands, traits, /*numResults=*/0,
requiresAccessGroup, requiresAliasAnalysis,
- /*requiresFastMath=*/0, immArgPositions, immArgAttrNames>;
+ /*requiresFastMath=*/0, immArgPositions, immArgAttrNames,
+ opBundleSizesAttrName, opBundleTagsAttrName>;
// Base class for LLVM intrinsic operations returning one result. Places the
// intrinsic into the LLVM dialect and prefixes its name with "intr.". This is
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index 000d92f9ea3bcb..d388de3960f2b2 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -559,11 +559,7 @@ def LLVM_InvokeOp : LLVM_Op<"invoke", [
VariadicOfVariadic<LLVM_Type,
"op_bundle_sizes">:$op_bundle_operands,
DenseI32ArrayAttr:$op_bundle_sizes,
- DefaultValuedProperty<
- ArrayProperty<StringProperty, "operand bundle tags">,
- "ArrayRef<std::string>{}",
- "SmallVector<std::string>{}"
- >:$op_bundle_tags);
+ OptionalAttr<ArrayAttr>:$op_bundle_tags);
let results = (outs Optional<LLVM_Type>:$result);
let successors = (successor AnySuccessor:$normalDest,
AnySuccessor:$unwindDest);
@@ -678,11 +674,7 @@ def LLVM_CallOp : LLVM_MemAccessOpBase<"call",
VariadicOfVariadic<LLVM_Type,
"op_bundle_sizes">:$op_bundle_operands,
DenseI32ArrayAttr:$op_bundle_sizes,
- DefaultValuedProperty<
- ArrayProperty<StringProperty, "operand bundle tags">,
- "ArrayRef<std::string>{}",
- "SmallVector<std::string>{}"
- >:$op_bundle_tags);
+ OptionalAttr<ArrayAttr>:$op_bundle_tags);
// Append the aliasing related attributes defined in LLVM_MemAccessOpBase.
let arguments = !con(args, aliasAttrs);
let results = (outs Optional<LLVM_Type>:$result);
@@ -1930,11 +1922,7 @@ def LLVM_CallIntrinsicOp
VariadicOfVariadic<LLVM_Type,
"op_bundle_sizes">:$op_bundle_operands,
DenseI32ArrayAttr:$op_bundle_sizes,
- DefaultValuedProperty<
- ArrayProperty<StringProperty, "operand bundle tags">,
- "ArrayRef<std::string>{}",
- "SmallVector<std::string>{}"
- >:$op_bundle_tags);
+ OptionalAttr<ArrayAttr>:$op_bundle_tags);
let results = (outs Optional<LLVM_Type>:$results);
let llvmBuilder = [{
return convertCallLLVMIntrinsicOp(op, builder, moduleTranslation);
diff --git a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
index 436675793062eb..a2cf065bb9eb76 100644
--- a/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
+++ b/mlir/include/mlir/Target/LLVMIR/ModuleImport.h
@@ -239,8 +239,11 @@ class ModuleImport {
/// corresponding MLIR attribute names.
LogicalResult
convertIntrinsicArguments(ArrayRef<llvm::Value *> values,
+ ArrayRef<llvm::OperandBundleUse> opBundles,
ArrayRef<unsigned> immArgPositions,
ArrayRef<StringLiteral> immArgAttrNames,
+ StringLiteral opBundleSizesAttrName,
+ StringLiteral opBundleTagsAttrName,
SmallVectorImpl<Value> &valuesOut,
SmallVectorImpl<NamedAttribute> &attrsOut);
diff --git a/mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h b/mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
index 3c85338bc642f6..4a8503ceb4cd9c 100644
--- a/mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
+++ b/mlir/include/mlir/Target/LLVMIR/ModuleTranslation.h
@@ -431,8 +431,8 @@ llvm::CallInst *createIntrinsicCall(
llvm::IRBuilderBase &builder, ModuleTranslation &moduleTranslation,
Operation *intrOp, llvm::Intrinsic::ID intrinsic, unsigned numResults,
ArrayRef<unsigned> overloadedResults, ArrayRef<unsigned> overloadedOperands,
- ArrayRef<unsigned> immArgPositions,
- ArrayRef<StringLiteral> immArgAttrNames);
+ ArrayRef<unsigned> immArgPositions, ArrayRef<StringLiteral> immArgAttrNames,
+ StringLiteral opBundleSizesAttrName, StringLiteral opBundleTagsAttrName);
} // namespace detail
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 006d412936a337..967aafd1dbc86e 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -241,13 +241,18 @@ static void printOneOpBundle(OpAsmPrinter &p, OperandRange operands,
static void printOpBundles(OpAsmPrinter &p, Operation *op,
OperandRangeRange opBundleOperands,
TypeRangeRange opBundleOperandTypes,
- ArrayRef<std::string> opBundleTags) {
+ std::optional<ArrayAttr> opBundleTags) {
+ if (opBundleOperands.empty())
+ return;
+ assert(opBundleTags && "expect operand bundle tags");
+
p << "[";
llvm::interleaveComma(
- llvm::zip(opBundleOperands, opBundleOperandTypes, opBundleTags), p,
+ llvm::zip(opBundleOperands, opBundleOperandTypes, *opBundleTags), p,
[&p](auto bundle) {
+ auto bundleTag = llvm::cast<StringAttr>(std::get<2>(bundle)).getValue();
printOneOpBundle(p, std::get<0>(bundle), std::get<1>(bundle),
- std::get<2>(bundle));
+ bundleTag);
});
p << "]";
}
@@ -256,7 +261,7 @@ static ParseResult parseOneOpBundle(
OpAsmParser &p,
SmallVector<SmallVector<OpAsmParser::UnresolvedOperand>> &opBundleOperands,
SmallVector<SmallVector<Type>> &opBundleOperandTypes,
- SmallVector<std::string> &opBundleTags) {
+ SmallVector<Attribute> &opBundleTags) {
SMLoc currentParserLoc = p.getCurrentLocation();
SmallVector<OpAsmParser::UnresolvedOperand> operands;
SmallVector<Type> types;
@@ -276,7 +281,7 @@ static ParseResult parseOneOpBundle(
opBundleOperands.push_back(std::move(operands));
opBundleOperandTypes.push_back(std::move(types));
- opBundleTags.push_back(std::move(tag));
+ opBundleTags.push_back(StringAttr::get(p.getContext(), tag));
return success();
}
@@ -285,16 +290,17 @@ static std::optional<ParseResult> parseOpBundles(
OpAsmParser &p,
SmallVector<SmallVector<OpAsmParser::UnresolvedOperand>> &opBundleOperands,
SmallVector<SmallVector<Type>> &opBundleOperandTypes,
- SmallVector<std::string> &opBundleTags) {
+ ArrayAttr &opBundleTags) {
if (p.parseOptionalLSquare())
return std::nullopt;
if (succeeded(p.parseOptionalRSquare()))
return success();
+ SmallVector<Attribute> opBundleTagAttrs;
auto bundleParser = [&] {
return parseOneOpBundle(p, opBundleOperands, opBundleOperandTypes,
- opBundleTags);
+ opBundleTagAttrs);
};
if (p.parseCommaSeparatedList(bundleParser))
return failure();
@@ -302,6 +308,8 @@ static std::optional<ParseResult> parseOpBundles(
if (p.parseRSquare())
return failure();
+ opBundleTags = ArrayAttr::get(p.getContext(), opBundleTagAttrs);
+
return success();
}
@@ -1039,7 +1047,7 @@ void CallOp::build(OpBuilder &builder, OperationState &state, TypeRange results,
/*CConv=*/nullptr, /*TailCallKind=*/nullptr,
/*memory_effects=*/nullptr,
/*convergent=*/nullptr, /*no_unwind=*/nullptr, /*will_return=*/nullptr,
- /*op_bundle_operands=*/{}, /*op_bundle_tags=*/std::nullopt,
+ /*op_bundle_operands=*/{}, /*op_bundle_tags=*/{},
/*access_groups=*/nullptr, /*alias_scopes=*/nullptr,
/*noalias_scopes=*/nullptr, /*tbaa=*/nullptr);
}
@@ -1066,7 +1074,7 @@ void CallOp::build(OpBuilder &builder, OperationState &state,
/*TailCallKind=*/nullptr, /*memory_effects=*/nullptr,
/*convergent=*/nullptr,
/*no_unwind=*/nullptr, /*will_return=*/nullptr,
- /*op_bundle_operands=*/{}, /*op_bundle_tags=*/std::nullopt,
+ /*op_bundle_operands=*/{}, /*op_bundle_tags=*/{},
/*access_groups=*/nullptr,
/*alias_scopes=*/nullptr, /*noalias_scopes=*/nullptr, /*tbaa=*/nullptr);
}
@@ -1079,7 +1087,7 @@ void CallOp::build(OpBuilder &builder, OperationState &state,
/*fastmathFlags=*/nullptr, /*branch_weights=*/nullptr,
/*CConv=*/nullptr, /*TailCallKind=*/nullptr, /*memory_effects=*/nullptr,
/*convergent=*/nullptr, /*no_unwind=*/nullptr, /*will_return=*/nullptr,
- /*op_bundle_operands=*/{}, /*op_bundle_tags=*/std::nullopt,
+ /*op_bundle_operands=*/{}, /*op_bundle_tags=*/{},
/*access_groups=*/nullptr, /*alias_scopes=*/nullptr,
/*noalias_scopes=*/nullptr, /*tbaa=*/nullptr);
}
@@ -1092,7 +1100,7 @@ void CallOp::build(OpBuilder &builder, OperationState &state, LLVMFuncOp func,
/*fastmathFlags=*/nullptr, /*branch_weights=*/nullptr,
/*CConv=*/nullptr, /*TailCallKind=*/nullptr, /*memory_effects=*/nullptr,
/*convergent=*/nullptr, /*no_unwind=*/nullptr, /*will_return=*/nullptr,
- /*op_bundle_operands=*/{}, /*op_bundle_tags=*/std::nullopt,
+ /*op_bundle_operands=*/{}, /*op_bundle_tags=*/{},
/*access_groups=*/nullptr, /*alias_scopes=*/nullptr,
/*noalias_scopes=*/nullptr, /*tbaa=*/nullptr);
}
@@ -1192,12 +1200,21 @@ LogicalResult verifyCallOpVarCalleeType(OpTy callOp) {
template <typename OpType>
static LogicalResult verifyOperandBundles(OpType &op) {
OperandRangeRange opBundleOperands = op.getOpBundleOperands();
- ArrayRef<std::string> opBundleTags = op.getOpBundleTags();
+ std::optional<ArrayAttr> opBundleTags = op.getOpBundleTags();
- if (opBundleTags.size() != opBundleOperands.size())
+ if (opBundleTags) {
+ for (Attribute tagAttr : *opBundleTags) {
+ if (!llvm::isa<StringAttr>(tagAttr))
+ return op.emitError("operand bundle tag must be a StringAttr");
+ }
+ }
+
+ size_t numOpBundles = opBundleOperands.size();
+ size_t numOpBundleTags = opBundleTags ? opBundleTags->size() : 0;
+ if (numOpBundles != numOpBundleTags)
return op.emitError("expected ")
- << opBundleOperands.size()
- << " operand bundle tags, but actually got " << opBundleTags.size();
+ << numOpBundles << " operand bundle tags, but actually got "
+ << numOpBundleTags;
return success();
}
@@ -1329,7 +1346,8 @@ void CallOp::print(OpAsmPrinter &p) {
{getCalleeAttrName(), getTailCallKindAttrName(),
getVarCalleeTypeAttrName(), getCConvAttrName(),
getOperandSegmentSizesAttrName(),
- getOpBundleSizesAttrName()});
+ getOpBundleSizesAttrName(),
+ getOpBundleTagsAttrName()});
p << " : ";
if (!isDirect)
@@ -1437,7 +1455,7 @@ ParseResult CallOp::parse(OpAsmParser &parser, OperationState &result) {
SmallVector<OpAsmParser::UnresolvedOperand> operands;
SmallVector<SmallVector<OpAsmParser::UnresolvedOperand>> opBundleOperands;
SmallVector<SmallVector<Type>> opBundleOperandTypes;
- SmallVector<std::string> opBundleTags;
+ ArrayAttr opBundleTags;
// Default to C Calling Convention if no keyword is provided.
result.addAttribute(
@@ -1483,9 +1501,9 @@ ParseResult CallOp::parse(OpAsmParser &parser, OperationState &result) {
parser, opBundleOperands, opBundleOperandTypes, opBundleTags);
result && failed(*result))
return failure();
- if (!opBundleTags.empty())
- result.getOrAddProperties<CallOp::Properties>().op_bundle_tags =
- std::move(opBundleTags);
+ if (opBundleTags && !opBundleTags.empty())
+ result.addAttribute(CallOp::getOpBundleTagsAttrName(result.name).getValue(),
+ opBundleTags);
if (parser.parseOptionalAttrDict(result.attributes))
return failure();
@@ -1525,8 +1543,7 @@ void InvokeOp::build(OpBuilder &builder, OperationState &state, LLVMFuncOp func,
auto calleeType = func.getFunctionType();
build(builder, state, getCallOpResultTypes(calleeType),
getCallOpVarCalleeType(calleeType), SymbolRefAttr::get(func), ops,
- normalOps, unwindOps, nullptr, nullptr, {}, std::nullopt, normal,
- unwind);
+ normalOps, unwindOps, nullptr, nullptr, {}, {}, normal, unwind);
}
void InvokeOp::build(OpBuilder &builder, OperationState &state, TypeRange tys,
@@ -1535,7 +1552,7 @@ void InvokeOp::build(OpBuilder &builder, OperationState &state, TypeRange tys,
ValueRange unwindOps) {
build(builder, state, tys,
/*var_callee_type=*/nullptr, callee, ops, normalOps, unwindOps, nullptr,
- nullptr, {}, std::nullopt, normal, unwind);
+ nullptr, {}, {}, normal, unwind);
}
void InvokeOp::build(OpBuilder &builder, OperationState &state,
@@ -1544,7 +1561,7 @@ void InvokeOp::build(OpBuilder &builder, OperationState &state,
Block *unwind, ValueRange unwindOps) {
build(builder, state, getCallOpResultTypes(calleeType),
getCallOpVarCalleeType(calleeType), callee, ops, normalOps, unwindOps,
- nullptr, nullptr, {}, std::nullopt, normal, unwind);
+ nullptr, nullptr, {}, {}, normal, unwind);
}
SuccessorOperands InvokeOp::getSuccessorOperands(unsigned index) {
@@ -1634,7 +1651,8 @@ void InvokeOp::print(OpAsmPrinter &p) {
p.printOptionalAttrDict((*this)->getAttrs(),
{getCalleeAttrName(), getOperandSegmentSizeAttr(),
getCConvAttrName(), getVarCalleeTypeAttrName(),
- getOpBundleSizesAttrName()});
+ getOpBundleSizesAttrName(),
+ getOpBundleTagsAttrName()});
p << " : ";
if (!isDirect)
@@ -1657,7 +1675,...
[truncated]
|
gysit
reviewed
Oct 14, 2024
mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMIRToLLVMTranslation.cpp
Outdated
Show resolved
Hide resolved
Lancern
force-pushed
the
mlir-llvm-assume-opbundle
branch
from
6e2a807
to
86c643f
Compare
Lancern
requested review from
banach-space,
dcaballe and
nicolasvasilache
as code owners
Lancern
force-pushed
the
mlir-llvm-assume-opbundle
branch
from
86c643f
to
eed529a
Compare
What are those burdens? Don't really care either way, but I'm just very curious 🙂 Might also be worth summarizing super shortly in the commit message |
gysit
approved these changes
Oct 14, 2024
The main problem with properties is that they are not available through
Updated. |
Lancern
force-pushed
the
mlir-llvm-assume-opbundle
branch
from
eed529a
to
4655bd6
Compare
Lancern
force-pushed
the
mlir-llvm-assume-opbundle
branch
from
4655bd6
to
57d749d
Compare
|
Any other comments on this PR? |
|
Looks good from my side! I believe you have now commit access? So feel free to land. |
|
Great, I'll merge now. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/129/builds/7685 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/10338 Here is the relevant piece of the build log for the reference |
|
Sorry but I'm not very familiar with how we deal with CI failures. Should I revert the patch and reland it after fixing? Or I just push another patch to fix it? |
|
Yes the correct way is to revert the commit briefly mentioning the failure in the commit message (maybe just put a link to the build bot). And then reland later. |
Lancern
added a commit
that referenced
this pull request
Oct 16, 2024
#112143)" This reverts commit d8fadad. The commit breaks the following CI builds: - ppc64le-mlir-rhel-clang: https://lab.llvm.org/buildbot/#/builders/129/builds/7685 - ppc64le-flang-rhel-clang: https://lab.llvm.org/buildbot/#/builders/157/builds/10338
|
Reverted, will reland later after fixing the build error. |
|
I guess you have to replace std::reduce since this is apparently not supported on power pc. std::accumulate seems to have uses in llvm so I supposes this maybe the way to go. When you reland the PR it makes sense to add the commit hash of the original commit and of the revert commit to the commit message for transparency. |
bricknerb
pushed a commit
to bricknerb/llvm-project
that referenced
this pull request
Oct 16, 2024
…112143) This patch adds operand bundle support for `llvm.intr.assume`. This patch actually contains two parts: - `llvm.intr.assume` now accepts operand bundle related attributes and operands. `llvm.intr.assume` does not take constraint on the operand bundles, but obviously only a few set of operand bundles are meaningful. I plan to add some of those (e.g. `aligned` and `separate_storage` are what interest me but other people may be interested in other operand bundles as well) in future patches. - The definitions of `llvm.call`, `llvm.invoke`, and `llvm.call_intrinsic` actually define `op_bundle_tags` as an operation property. It turns out this approach would introduce some unnecessary burden if applied equally to the intrinsic operations because properties are not available through `Operation *` but we have to operate on `Operation *` during the import/export of intrinsics, so this PR changes it from a property to an array attribute.
bricknerb
pushed a commit
to bricknerb/llvm-project
that referenced
this pull request
Oct 16, 2024
llvm#112143)" This reverts commit d8fadad. The commit breaks the following CI builds: - ppc64le-mlir-rhel-clang: https://lab.llvm.org/buildbot/#/builders/129/builds/7685 - ppc64le-flang-rhel-clang: https://lab.llvm.org/buildbot/#/builders/157/builds/10338
Lancern
added a commit
that referenced
this pull request
Oct 16, 2024
This patch adds operand bundle support for `llvm.intr.assume`. This patch actually contains two parts: - `llvm.intr.assume` now accepts operand bundle related attributes and operands. `llvm.intr.assume` does not take constraint on the operand bundles, but obviously only a few set of operand bundles are meaningful. I plan to add some of those (e.g. `aligned` and `separate_storage` are what interest me but other people may be interested in other operand bundles as well) in future patches. - The definitions of `llvm.call`, `llvm.invoke`, and `llvm.call_intrinsic` actually define `op_bundle_tags` as an operation property. It turns out this approach would introduce some unnecessary burden if applied equally to the intrinsic operations because properties are not available through `Operation *` but we have to operate on `Operation *` during the import/export of intrinsics, so this PR changes it from a property to an array attribute. This patch relands commit d8fadad.
|
Just re-landed. Let's see if CI build could pass now. |
It passed. |
bricknerb
pushed a commit
to bricknerb/llvm-project
that referenced
this pull request
Oct 17, 2024
…112143) This patch adds operand bundle support for `llvm.intr.assume`. This patch actually contains two parts: - `llvm.intr.assume` now accepts operand bundle related attributes and operands. `llvm.intr.assume` does not take constraint on the operand bundles, but obviously only a few set of operand bundles are meaningful. I plan to add some of those (e.g. `aligned` and `separate_storage` are what interest me but other people may be interested in other operand bundles as well) in future patches. - The definitions of `llvm.call`, `llvm.invoke`, and `llvm.call_intrinsic` actually define `op_bundle_tags` as an operation property. It turns out this approach would introduce some unnecessary burden if applied equally to the intrinsic operations because properties are not available through `Operation *` but we have to operate on `Operation *` during the import/export of intrinsics, so this PR changes it from a property to an array attribute.
bricknerb
pushed a commit
to bricknerb/llvm-project
that referenced
this pull request
Oct 17, 2024
llvm#112143)" This reverts commit d8fadad. The commit breaks the following CI builds: - ppc64le-mlir-rhel-clang: https://lab.llvm.org/buildbot/#/builders/129/builds/7685 - ppc64le-flang-rhel-clang: https://lab.llvm.org/buildbot/#/builders/157/builds/10338
bricknerb
pushed a commit
to bricknerb/llvm-project
that referenced
this pull request
Oct 17, 2024
…112143) This patch adds operand bundle support for `llvm.intr.assume`. This patch actually contains two parts: - `llvm.intr.assume` now accepts operand bundle related attributes and operands. `llvm.intr.assume` does not take constraint on the operand bundles, but obviously only a few set of operand bundles are meaningful. I plan to add some of those (e.g. `aligned` and `separate_storage` are what interest me but other people may be interested in other operand bundles as well) in future patches. - The definitions of `llvm.call`, `llvm.invoke`, and `llvm.call_intrinsic` actually define `op_bundle_tags` as an operation property. It turns out this approach would introduce some unnecessary burden if applied equally to the intrinsic operations because properties are not available through `Operation *` but we have to operate on `Operation *` during the import/export of intrinsics, so this PR changes it from a property to an array attribute. This patch relands commit d8fadad.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds operand bundle support for
llvm.intr.assume.This PR actually contains two parts:
llvm.intr.assumenow accepts operand bundle related attributes and operands.llvm.intr.assumedoes not take constraint on the operand bundles, but obviously only a few set of operand bundles are meaningful. I plan to add some of those (e.g.alignedandseparate_storageare what interest me but other people may be interested in other operand bundles as well) in future patches.The definitions of
llvm.call,llvm.invoke, andllvm.call_intrinsicactually defineop_bundle_tagsas an operation property. It turns out this approach would introduce some unnecessary burden if applied equally to the intrinsic operations because properties are not available throughOperation *but we have to operate onOperation *during the import/export of intrinsics, so this PR changes it from a property to an array attribute.