Skip to content

Commit d9b0a9b

Browse files
JanekvOJanekvO
committed
Feedback, use optional for MCExpr instead of INT64_MIN
1 parent 7eaae11 commit d9b0a9b

File tree

5 files changed

+68
-35
lines changed

5 files changed

+68
-35
lines changed

‎llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8287,29 +8287,6 @@ void AMDGPUAsmParser::onBeginOfFile() {
82878287
bool AMDGPUAsmParser::parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) {
82888288
using AGVK = AMDGPUVariadicMCExpr::AMDGPUVariadicKind;
82898289

8290-
auto ParseVariadicExpr = [&](AGVK Kind, const MCExpr *&Result,
8291-
SMLoc &EndLoc) {
8292-
SmallVector<const MCExpr *, 4> Exprs;
8293-
while (true) {
8294-
if (trySkipToken(AsmToken::RParen)) {
8295-
if (Exprs.empty()) {
8296-
Error(getToken().getLoc(), "empty max/or expression");
8297-
return true;
8298-
}
8299-
Result = AMDGPUVariadicMCExpr::create(Kind, Exprs, getContext());
8300-
return false;
8301-
}
8302-
const MCExpr *Expr;
8303-
if (getParser().parseExpression(Expr, EndLoc))
8304-
return true;
8305-
Exprs.push_back(Expr);
8306-
if (!trySkipToken(AsmToken::Comma) && !isToken(AsmToken::RParen)) {
8307-
Error(getToken().getLoc(), "unexpected token in max/or expression");
8308-
return true;
8309-
}
8310-
}
8311-
};
8312-
83138290
if (isToken(AsmToken::Identifier)) {
83148291
StringRef TokenId = getTokenStr();
83158292
AGVK VK = StringSwitch<AGVK>(TokenId)
@@ -8318,9 +8295,29 @@ bool AMDGPUAsmParser::parsePrimaryExpr(const MCExpr *&Res, SMLoc &EndLoc) {
83188295
.Default(AGVK::AGVK_None);
83198296

83208297
if (VK != AGVK::AGVK_None && peekToken().is(AsmToken::LParen)) {
8298+
SmallVector<const MCExpr *, 4> Exprs;
83218299
lex(); // Eat 'max'/'or'
83228300
lex(); // Eat '('
8323-
return ParseVariadicExpr(VK, Res, EndLoc);
8301+
while (true) {
8302+
if (trySkipToken(AsmToken::RParen)) {
8303+
if (Exprs.empty()) {
8304+
Error(getToken().getLoc(),
8305+
"empty " + Twine(TokenId) + " expression");
8306+
return true;
8307+
}
8308+
Res = AMDGPUVariadicMCExpr::create(VK, Exprs, getContext());
8309+
return false;
8310+
}
8311+
const MCExpr *Expr;
8312+
if (getParser().parseExpression(Expr, EndLoc))
8313+
return true;
8314+
Exprs.push_back(Expr);
8315+
if (!trySkipToken(AsmToken::Comma) && !isToken(AsmToken::RParen)) {
8316+
Error(getToken().getLoc(),
8317+
"unexpected token in " + Twine(TokenId) + " expression");
8318+
return true;
8319+
}
8320+
}
83248321
}
83258322
}
83268323
return getParser().parsePrimaryExpr(Res, EndLoc, nullptr);

‎llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
#include "llvm/MC/MCValue.h"
1414
#include "llvm/Support/Allocator.h"
1515
#include "llvm/Support/raw_ostream.h"
16+
#include <optional>
1617

1718
using namespace llvm;
1819

@@ -22,7 +23,7 @@ AMDGPUVariadicMCExpr::create(AMDGPUVariadicKind Kind,
2223
return new (Ctx) AMDGPUVariadicMCExpr(Kind, Args);
2324
}
2425

25-
const MCExpr *AMDGPUVariadicMCExpr::getSubExpr(size_t index) const {
26+
const MCExpr *AMDGPUVariadicMCExpr::GetSubExpr(size_t index) const {
2627
assert(index < Args.size() &&
2728
"Indexing out of bounds AMDGPUVariadicMCExpr sub-expr");
2829
return Args[index];
@@ -50,7 +51,7 @@ void AMDGPUVariadicMCExpr::printImpl(raw_ostream &OS,
5051

5152
bool AMDGPUVariadicMCExpr::evaluateAsRelocatableImpl(
5253
MCValue &Res, const MCAsmLayout *Layout, const MCFixup *Fixup) const {
53-
int64_t Total = INT64_MIN;
54+
std::optional<int64_t> Total = {};
5455

5556
auto Op = [this](int64_t Arg1, int64_t Arg2) -> int64_t {
5657
switch (Kind) {
@@ -69,12 +70,12 @@ bool AMDGPUVariadicMCExpr::evaluateAsRelocatableImpl(
6970
!ArgRes.isAbsolute())
7071
return false;
7172

72-
if (Total == INT64_MIN)
73+
if (!Total.has_value())
7374
Total = ArgRes.getConstant();
74-
Total = Op(Total, ArgRes.getConstant());
75+
Total = Op(*Total, ArgRes.getConstant());
7576
}
7677

77-
Res = MCValue::get(Total);
78+
Res = MCValue::get(*Total);
7879
return true;
7980
}
8081

‎llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUMCExpr.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ namespace llvm {
2222
/// - (logic) or
2323
/// - max
2424
///
25+
/// \note If the 'or'/'max' operations are provided only a single argument, the
26+
/// operation will act as a no-op and simply resolve as the provided argument.
27+
///
2528
class AMDGPUVariadicMCExpr : public MCTargetExpr {
2629
public:
2730
enum AMDGPUVariadicKind { AGVK_None, AGVK_Or, AGVK_Max };
@@ -51,17 +54,18 @@ class AMDGPUVariadicMCExpr : public MCTargetExpr {
5154
}
5255

5356
AMDGPUVariadicKind getKind() const { return Kind; }
54-
const MCExpr *getSubExpr(size_t index) const;
57+
const MCExpr *GetSubExpr(size_t index) const;
5558

5659
void printImpl(raw_ostream &OS, const MCAsmInfo *MAI) const override;
5760
bool evaluateAsRelocatableImpl(MCValue &Res, const MCAsmLayout *Layout,
5861
const MCFixup *Fixup) const override;
5962
void visitUsedExpr(MCStreamer &Streamer) const override;
6063
MCFragment *findAssociatedFragment() const override;
64+
void fixELFSymbolsInTLSFixups(MCAssembler &) const override{};
65+
6166
static bool classof(const MCExpr *E) {
6267
return E->getKind() == MCExpr::Target;
6368
}
64-
void fixELFSymbolsInTLSFixups(MCAssembler &) const override{};
6569
};
6670

6771
} // end namespace llvm

‎llvm/test/MC/AMDGPU/mcexpr_amd.s

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
// OBJDUMP-NEXT: 0000000000000001 l *ABS* 0000000000000000 one
88
// OBJDUMP-NEXT: 0000000000000002 l *ABS* 0000000000000000 two
99
// OBJDUMP-NEXT: 0000000000000003 l *ABS* 0000000000000000 three
10+
// OBJDUMP-NEXT: 7fffffffffffffff l *ABS* 0000000000000000 i64_max
11+
// OBJDUMP-NEXT: 8000000000000000 l *ABS* 0000000000000000 i64_min
1012
// OBJDUMP-NEXT: 0000000000000005 l *ABS* 0000000000000000 max_expression_all
1113
// OBJDUMP-NEXT: 0000000000000005 l *ABS* 0000000000000000 five
1214
// OBJDUMP-NEXT: 0000000000000004 l *ABS* 0000000000000000 four
@@ -21,6 +23,12 @@
2123
// OBJDUMP-NEXT: 0000000000000003 l *ABS* 0000000000000000 max_with_subexpr
2224
// OBJDUMP-NEXT: 0000000000000006 l *ABS* 0000000000000000 max_as_subexpr
2325
// OBJDUMP-NEXT: 0000000000000005 l *ABS* 0000000000000000 max_recursive_subexpr
26+
// OBJDUMP-NEXT: 7fffffffffffffff l *ABS* 0000000000000000 max_expr_one_max
27+
// OBJDUMP-NEXT: 7fffffffffffffff l *ABS* 0000000000000000 max_expr_two_max
28+
// OBJDUMP-NEXT: 7fffffffffffffff l *ABS* 0000000000000000 max_expr_three_max
29+
// OBJDUMP-NEXT: 8000000000000000 l *ABS* 0000000000000000 max_expr_one_min
30+
// OBJDUMP-NEXT: 0000000000000003 l *ABS* 0000000000000000 max_expr_two_min
31+
// OBJDUMP-NEXT: 0000000000989680 l *ABS* 0000000000000000 max_expr_three_min
2432
// OBJDUMP-NEXT: 0000000000000001 l *ABS* 0000000000000000 or_expression_all
2533
// OBJDUMP-NEXT: 0000000000000001 l *ABS* 0000000000000000 or_expression_two
2634
// OBJDUMP-NEXT: 0000000000000001 l *ABS* 0000000000000000 or_expression_one
@@ -36,11 +44,15 @@
3644
// ASM: .set one, 1
3745
// ASM: .set two, 2
3846
// ASM: .set three, 3
47+
// ASM: .set i64_max, 9223372036854775807
48+
// ASM: .set i64_min, -9223372036854775808
3949

4050
.set zero, 0
4151
.set one, 1
4252
.set two, 2
4353
.set three, 3
54+
.set i64_max, 0x7FFFFFFFFFFFFFFF
55+
.set i64_min, 0x8000000000000000
4456

4557
// ASM: .set max_expression_all, max(1, 2, five, 3, four)
4658
// ASM: .set max_expression_two, 2
@@ -69,6 +81,22 @@
6981
.set max_as_subexpr, 1 + max(4, 3, five)
7082
.set max_recursive_subexpr, max(max(one, four), three, max_expression_all)
7183

84+
// ASM: .set max_expr_one_max, 9223372036854775807
85+
// ASM: .set max_expr_two_max, max(9223372036854775807, five)
86+
// ASM: .set max_expr_three_max, max(9223372036854775807, five, 10000000)
87+
88+
.set max_expr_one_max, max(i64_max)
89+
.set max_expr_two_max, max(i64_max, five)
90+
.set max_expr_three_max, max(i64_max, five, 10000000)
91+
92+
// ASM: .set max_expr_one_min, -9223372036854775808
93+
// ASM: .set max_expr_two_min, 3
94+
// ASM: .set max_expr_three_min, 10000000
95+
96+
.set max_expr_one_min, max(i64_min)
97+
.set max_expr_two_min, max(i64_min, three)
98+
.set max_expr_three_min, max(i64_min, three, 10000000)
99+
72100
// ASM: .set or_expression_all, or(1, 2, five, 3, four)
73101
// ASM: .set or_expression_two, 1
74102
// ASM: .set or_expression_one, 1

‎llvm/test/MC/AMDGPU/mcexpr_amd_err.s

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,29 @@
11
// RUN: not llvm-mc -triple amdgcn-amd-amdhsa %s 2>&1 | FileCheck --check-prefix=ASM %s
22

3-
// ASM: error: empty max/or expression
3+
// ASM: error: empty max expression
4+
// ASM: error: missing expression
5+
// ASM: error: empty or expression
46
// ASM: error: missing expression
57
// ASM: error: unknown token in expression
68
// ASM: error: missing expression
7-
// ASM: error: unexpected token in max/or expression
9+
// ASM: error: unexpected token in max expression
810
// ASM: error: missing expression
911
// ASM: error: unknown token in expression
1012
// ASM: error: missing expression
11-
// ASM: error: unexpected token in max/or expression
13+
// ASM: error: unexpected token in or expression
1214
// ASM: error: missing expression
1315

1416
.set one, 1
1517
.set two, 2
1618
.set three, 3
1719

1820
.set max_empty, max()
21+
.set or_empty, or()
1922
.set max_post_aux_comma, max(one,)
2023
.set max_pre_aux_comma, max(,one)
2124
.set max_missing_paren, max(two
2225
.set max_expression_one, max(three, four,
23-
.set max_expression_one, max(four, five
26+
.set or_expression_one, or(four, five
2427

2528
.set four, 4
2629
.set five, 5

0 commit comments

Comments
 (0)