[AMDGPU] Added min operation for MCExprs#199746
Conversation
|
Hello @JoshuaGrindstaff 👋 Thank you for submitting a Pull Request (PR) to the LLVM Project. Since this is your first PR, here are a few useful links covering our main contribution policies and review practices.
Please reply to this message to confirm that you have read these policies, especially the LLVM AI Tool Use Policy, and that any AI tool usage has been noted in the PR description. Frequently asked questionsHow do I add reviewers? This PR will be automatically labeled, and the relevant teams will be notified. For some parts of the project, reviewers may also be added automatically. You can also add reviewers manually using the Reviewers section on this page. If you cannot use that section, it is probably because you do not have write permissions for the repository. In that case, you can request a review by tagging reviewers in a comment using What if there are no comments? If you have not received any comments on your PR after a week, you can request a review by pinging the PR with a comment such as “Ping”. The common courtesy ping rate is once a week. Please remember that you are asking for volunteer time from other developers. Are any special GitHub settings required to contribute to LLVM? We only require contributors to have a public email address associated with their GitHub commits, see this section of LLVM Developer Policy for details. If you have questions, feel free to leave a comment on this PR, or ask on LLVM Discord or LLVM Discourse. Thank you, |
|
@llvm/pr-subscribers-backend-amdgpu Author: JoshuaGrindstaff ChangesThe min operation is needed in MC Expressions for a future change that caps the max number of registers used for indirect calls. Full diff: https://github.com/llvm/llvm-project/pull/199746.diff 6 Files Affected:
diff --git a/llvm/docs/AMDGPUUsage.rst b/llvm/docs/AMDGPUUsage.rst
index f2bda0d3ec0b3..fed121a7c02c2 100644
--- a/llvm/docs/AMDGPUUsage.rst
+++ b/llvm/docs/AMDGPUUsage.rst
@@ -2529,6 +2529,9 @@ As part of the AMDGPU MC layer, AMDGPU provides the following target-specific
``max(arg, ...)`` 1 or more Variadic signed operation that returns the maximum
value of all its arguments.
+ ``min(arg, ...)`` 1 or more Variadic signed operation that returns the minimum
+ value of all its arguments
+
``or(arg, ...)`` 1 or more Variadic signed operation that returns the bitwise-or
result of all its arguments.
diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index bcfb105137af8..0a30f09fdeb37 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -9344,7 +9344,8 @@ void AMDGPUAsmParser::onBeginOfFile() {
/// Parse AMDGPU specific expressions.
///
/// expr ::= or(expr, ...) |
-/// max(expr, ...)
+/// max(expr, ...) |
+/// min(expr, ...)
///
bool AMDGPUAsmParser::parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) {
using AGVK = AMDGPUMCExpr::VariantKind;
@@ -9353,6 +9354,7 @@ bool AMDGPUAsmParser::parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) {
StringRef TokenId = getTokenStr();
AGVK VK = StringSwitch<AGVK>(TokenId)
.Case("max", AGVK::AGVK_Max)
+ .Case("min", AGVK::AGVK_Min)
.Case("or", AGVK::AGVK_Or)
.Case("extrasgprs", AGVK::AGVK_ExtraSGPRs)
.Case("totalnumvgprs", AGVK::AGVK_TotalNumVGPRs)
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
index 08871f43a42b5..90ac9147b9eeb 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp
@@ -65,6 +65,9 @@ void AMDGPUMCExpr::printImpl(raw_ostream &OS, const MCAsmInfo *MAI) const {
case AGVK_Max:
OS << "max(";
break;
+ case AGVK_Min:
+ OS << "min(";
+ break;
case AGVK_ExtraSGPRs:
OS << "extrasgprs(";
break;
@@ -103,6 +106,8 @@ static int64_t op(AMDGPUMCExpr::VariantKind Kind, int64_t Arg1, int64_t Arg2) {
return std::max(Arg1, Arg2);
case AMDGPUMCExpr::AGVK_Or:
return Arg1 | Arg2;
+ case AMDGPUMCExpr::AGVK_Min:
+ return std::min(Arg1, Arg2);
}
}
@@ -499,6 +504,16 @@ static void targetOpKnownBitsMapHelper(const MCExpr *Expr, KnownBitsMap &KBM,
KBM[Expr] = std::move(KB);
return;
}
+ case AMDGPUMCExpr::VariantKind::AGVK_Min: {
+ knownBitsMapHelper(AGVK->getSubExpr(0), KBM, Depth + 1);
+ KnownBits KB = KBM[AGVK->getSubExpr(0)];
+ for (const MCExpr *Arg : AGVK->getArgs()) {
+ knownBitsMapHelper(Arg, KBM, Depth + 1);
+ KB = KnownBits::umin(KB, KBM[Arg]);
+ }
+ KBM[Expr] = std::move(KB);
+ return;
+ }
case AMDGPUMCExpr::VariantKind::AGVK_ExtraSGPRs:
case AMDGPUMCExpr::VariantKind::AGVK_TotalNumVGPRs:
case AMDGPUMCExpr::VariantKind::AGVK_AlignTo:
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
index 66b6fdb4b0042..33a8f5d21af4c 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h
@@ -24,8 +24,9 @@ enum class LitModifier { None, Lit, Lit64 };
/// operations are:
/// - (bitwise) or
/// - max
+/// - min
///
-/// \note If the 'or'/'max' operations are provided only a single argument, the
+/// \note If the 'or'/'max'/'min' operations are provided only a single argument, the
/// operation will act as a no-op and simply resolve as the provided argument.
///
class AMDGPUMCExpr : public MCTargetExpr {
@@ -41,6 +42,7 @@ class AMDGPUMCExpr : public MCTargetExpr {
AGVK_InstPrefSize,
AGVK_Lit,
AGVK_Lit64,
+ AGVK_Min,
};
// Relocation specifiers.
@@ -85,6 +87,10 @@ class AMDGPUMCExpr : public MCTargetExpr {
MCContext &Ctx) {
return create(VariantKind::AGVK_Max, Args, Ctx);
}
+ static const AMDGPUMCExpr *createMin(ArrayRef<const MCExpr *> Args,
+ MCContext &Ctx) {
+ return create(VariantKind::AGVK_Min, Args, Ctx);
+ }
static const AMDGPUMCExpr *createExtraSGPRs(const MCExpr *VCCUsed,
const MCExpr *FlatScrUsed,
diff --git a/llvm/test/MC/AMDGPU/mcexpr_amd.s b/llvm/test/MC/AMDGPU/mcexpr_amd.s
index d7340bb5fd2ed..4423d56f50397 100644
--- a/llvm/test/MC/AMDGPU/mcexpr_amd.s
+++ b/llvm/test/MC/AMDGPU/mcexpr_amd.s
@@ -17,7 +17,6 @@
// OBJDUMP-NEXT: 000000000000000a l *ABS* 0000000000000000 max_literals
// OBJDUMP-NEXT: 000000000000000f l *ABS* 0000000000000000 max_with_max_sym
// OBJDUMP-NEXT: 000000000000000f l *ABS* 0000000000000000 max
-// OBJDUMP-NEXT: ffffffffffffffff l *ABS* 0000000000000000 neg_one
// OBJDUMP-NEXT: ffffffffffffffff l *ABS* 0000000000000000 max_neg_numbers
// OBJDUMP-NEXT: ffffffffffffffff l *ABS* 0000000000000000 max_neg_number
// OBJDUMP-NEXT: 0000000000000003 l *ABS* 0000000000000000 max_with_subexpr
@@ -29,6 +28,24 @@
// OBJDUMP-NEXT: 8000000000000000 l *ABS* 0000000000000000 max_expr_one_min
// OBJDUMP-NEXT: 0000000000000003 l *ABS* 0000000000000000 max_expr_two_min
// OBJDUMP-NEXT: 0000000000989680 l *ABS* 0000000000000000 max_expr_three_min
+// OBJDUMP-NEXT: 0000000000000001 l *ABS* 0000000000000000 min_expression_all
+// OBJDUMP-NEXT: 0000000000000001 l *ABS* 0000000000000000 min_expression_two
+// OBJDUMP-NEXT: 0000000000000003 l *ABS* 0000000000000000 min_expression_one
+// OBJDUMP-NEXT: 0000000000000001 l *ABS* 0000000000000000 min_literals
+// OBJDUMP-NEXT: 0000000000000000 l *ABS* 0000000000000000 min_with_min_sym
+// OBJDUMP-NEXT: 0000000000000000 l *ABS* 0000000000000000 min
+// OBJDUMP-NEXT: ffffffffffffffff l *ABS* 0000000000000000 neg_one
+// OBJDUMP-NEXT: fffffffffffffffb l *ABS* 0000000000000000 min_neg_numbers
+// OBJDUMP-NEXT: ffffffffffffffff l *ABS* 0000000000000000 min_neg_number
+// OBJDUMP-NEXT: 0000000000000003 l *ABS* 0000000000000000 min_with_subexpr
+// OBJDUMP-NEXT: 0000000000000004 l *ABS* 0000000000000000 min_as_subexpr
+// OBJDUMP-NEXT: 0000000000000001 l *ABS* 0000000000000000 min_recursive_subexpr
+// OBJDUMP-NEXT: 7fffffffffffffff l *ABS* 0000000000000000 min_expr_one_max
+// OBJDUMP-NEXT: 0000000000000003 l *ABS* 0000000000000000 min_expr_two_max
+// OBJDUMP-NEXT: ffffffffff676980 l *ABS* 0000000000000000 min_expr_three_max
+// OBJDUMP-NEXT: 8000000000000000 l *ABS* 0000000000000000 min_expr_one_min
+// OBJDUMP-NEXT: 8000000000000000 l *ABS* 0000000000000000 min_expr_two_min
+// OBJDUMP-NEXT: 8000000000000000 l *ABS* 0000000000000000 min_expr_three_min
// OBJDUMP-NEXT: 0000000000000007 l *ABS* 0000000000000000 or_expression_all
// OBJDUMP-NEXT: 0000000000000003 l *ABS* 0000000000000000 or_expression_two
// OBJDUMP-NEXT: 0000000000000001 l *ABS* 0000000000000000 or_expression_one
@@ -97,6 +114,49 @@
.set max_expr_two_min, max(i64_min, three)
.set max_expr_three_min, max(i64_min, three, 10000000)
+// ASM: .set min_expression_all, min(1, 2, five, 3, four)
+// ASM: .set min_expression_two, 1
+// ASM: .set min_expression_one, 3
+// ASM: .set min_literals, 1
+// ASM: .set min_with_min_sym, min(min, 4, 3, 1, 2)
+
+.set min_expression_all, min(one, two, five, three, four)
+.set min_expression_two, min(one, three)
+.set min_expression_one, min(three)
+.set min_literals, min(1,2,3,4,5,6,7,8,9,10)
+.set min_with_min_sym, min(min, 4, 3, one, two)
+
+// ASM: .set min_neg_numbers, -5
+// ASM: .set min_neg_number, -1
+
+.set neg_one, -1
+.set min_neg_numbers, min(-5, -4, -3, -2, neg_one)
+.set min_neg_number, min(neg_one)
+
+// ASM: .set min_with_subexpr, 3
+// ASM: .set min_as_subexpr, 1+min(4, 3, five)
+// ASM: .set min_recursive_subexpr, min(min(1, four), 3, min_expression_all)
+
+.set min_with_subexpr, min(((one | 3) << 3) / 8)
+.set min_as_subexpr, 1 + min(4, 3, five)
+.set min_recursive_subexpr, min(min(one, four), three, min_expression_all)
+
+// ASM: .set min_expr_one_max, 9223372036854775807
+// ASM: .set min_expr_two_max, 3
+// ASM: .set min_expr_three_max, -10000000
+
+.set min_expr_one_max, min(i64_max)
+.set min_expr_two_max, min(i64_max, three)
+.set min_expr_three_max, min(i64_max, three, -10000000)
+
+// ASM: .set min_expr_one_min, -9223372036854775808
+// ASM: .set min_expr_two_min, min(-9223372036854775808, five)
+// ASM: .set min_expr_three_min, min(-9223372036854775808, five, 10000000)
+
+.set min_expr_one_min, min(i64_min)
+.set min_expr_two_min, min(i64_min, five)
+.set min_expr_three_min, min(i64_min, five, 10000000)
+
// ASM: .set or_expression_all, or(1, 2, five, 3, four)
// ASM: .set or_expression_two, 3
// ASM: .set or_expression_one, 1
@@ -127,4 +187,5 @@
.set four, 4
.set five, 5
.set max, 0xF
+.set min, 0x0
.set or, 0xFF
diff --git a/llvm/test/MC/AMDGPU/mcexpr_amd_err.s b/llvm/test/MC/AMDGPU/mcexpr_amd_err.s
index 834c6eee8c31f..e97479688c5fe 100644
--- a/llvm/test/MC/AMDGPU/mcexpr_amd_err.s
+++ b/llvm/test/MC/AMDGPU/mcexpr_amd_err.s
@@ -8,6 +8,10 @@
// ASM: :[[@LINE-1]]:{{[0-9]+}}: error: empty max expression
// ASM: :[[@LINE-2]]:{{[0-9]+}}: error: missing expression
+.set min_empty, min()
+// ASM: :[[@LINE-1]]:{{[0-9]+}}: error: empty min expression
+// ASM: :[[@LINE-2]]:{{[0-9]+}}: error: missing expression
+
.set or_empty, or()
// ASM: :[[@LINE-1]]:{{[0-9]+}}: error: empty or expression
// ASM: :[[@LINE-2]]:{{[0-9]+}}: error: missing expression
@@ -40,6 +44,10 @@
// ASM: :[[@LINE-1]]:{{[0-9]+}}: error: unexpected token in or expression
// ASM: :[[@LINE-2]]:{{[0-9]+}}: error: missing expression
+.set min_expression_one, min(four,five
+// ASM: :[[@LINE-1]]:{{[0-9]+}}: error: unexpected token in min expression
+// ASM: :[[@LINE-2]]:{{[0-9]+}}: error: missing expression
+
.set max_no_lparen, max four, five)
// ASM: :[[@LINE-1]]:{{[0-9]+}}: error: expected newline
@@ -49,5 +57,8 @@
.set max_rparen_only, max)
// ASM: :[[@LINE-1]]:{{[0-9]+}}: error: expected newline
+.set min_no_lparen, min four, five)
+// ASM: :[[@LINE-1]]:{{[0-9]+}}: error: expected newline
+
.set four, 4
.set five, 5
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
I have read those policies |
JoshuaGrindstaff
force-pushed
the
Adding_Min
branch
from
51b655b to
983d3a5
Compare
bcahoon
left a comment
bcahoon
left a comment
There was a problem hiding this comment.
LGTM, other than my question about using umin.
| KnownBits KB = KBM[AGVK->getSubExpr(0)]; | ||
| for (const MCExpr *Arg : AGVK->getArgs()) { | ||
| knownBitsMapHelper(Arg, KBM, Depth + 1); | ||
| KB = KnownBits::umin(KB, KBM[Arg]); |
There was a problem hiding this comment.
It's odd that this uses umin, when the definition described in AMGPUUsage.rst says the operator is for signed values. I'm sure this is because Max uses umax, but I'm not sure why that doesn't use smax instead.
2 participants
The min operation is needed in MC Expressions for a future change that caps the max number of registers used for indirect calls.