Skip to content

Commit a782fcf

Browse files
committed
[CodeGen] Use 128bits for LaneBitmask.
This follows on from the conversation on llvm#109797. FWIW, I've considered several alternatives to this patch; (1) Using APInt as storage type rather than 'uint64_t Mask[2]'. (1) makes the code a bit simpler to read, but APInt by default only allocates space for a 64-bit value and otherwise dynamically allocates a larger buffer to represent the larger value. Because we know the value is always 128bit, this extra dynamic allocation is undesirable. We also rarely need the full power of APInt since most of the tests are bitwise operations, so I made the choice to represent it as a uint64_t array instead, and only moving to/from APInt when this is necessary. Because it is inconvenient that increasing the BitWidth applies to all targets, I tried to see if there is a way to make the bitwidth dynamic, by: (2) Making the BitWidth dynamic per target by passing it to constructors. (3) Modification of (2) that describes 'none', 'all' and 'lane' with an an enum which doesn't require a BitWidth, until doing arithmetic with an explicit bit mask (which does have a BitWidth). Unfortunately both these approaches don't seem feasible. For (2) that is because it would require passing the TargetRegisterInfo/MCRegisterInfo to many places where this info is not available, where it needs to instantiates a LaneBitmask value. Approach (3) leads to other issues such as questions like 'what is the meaning of 'operator==' when one value is a mask and the other is a 'all' enum?' If we let 'operator==' discard the bitwidth such that a 64-bit all-true bitmask == LaneBitmask::all() (using 'all' enum), then we could end up in a situation where: X == LaneBitmask::all() && Y == LaneBitmask::all() but `X != Y`. I considered replacing the equality operators by methods that take a RegisterInfo pointer, but the LaneBitmask struct is used in STL containers which require a plain 'operator==' or 'operator<'. We could work around that by providing custom lambdas (that call the method with the TargetInfo pointer), but this just gets increasingly more hacky. Perhaps just using more bits isn't actually that bad in practice.
1 parent 2064087 commit a782fcf

File tree

12 files changed

+253
-84
lines changed

12 files changed

+253
-84
lines changed

llvm/include/llvm/CodeGen/RDFLiveness.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ namespace std {
4343

4444
template <> struct hash<llvm::rdf::detail::NodeRef> {
4545
std::size_t operator()(llvm::rdf::detail::NodeRef R) const {
46-
return std::hash<llvm::rdf::NodeId>{}(R.first) ^
47-
std::hash<llvm::LaneBitmask::Type>{}(R.second.getAsInteger());
46+
return llvm::hash_value<llvm::rdf::NodeId>(R.first) ^
47+
llvm::hash_value(R.second.getAsPair());
4848
}
4949
};
5050

llvm/include/llvm/CodeGen/RDFRegisters.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,8 +106,8 @@ struct RegisterRef {
106106
}
107107

108108
size_t hash() const {
109-
return std::hash<RegisterId>{}(Reg) ^
110-
std::hash<LaneBitmask::Type>{}(Mask.getAsInteger());
109+
return llvm::hash_value<RegisterId>(Reg) ^
110+
llvm::hash_value(Mask.getAsPair());
111111
}
112112

113113
static constexpr bool isRegId(unsigned Id) {

llvm/include/llvm/MC/LaneBitmask.h

Lines changed: 97 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -29,72 +29,120 @@
2929
#ifndef LLVM_MC_LANEBITMASK_H
3030
#define LLVM_MC_LANEBITMASK_H
3131

32+
#include "llvm/ADT/APInt.h"
33+
#include "llvm/ADT/SmallString.h"
3234
#include "llvm/Support/Compiler.h"
3335
#include "llvm/Support/Format.h"
3436
#include "llvm/Support/MathExtras.h"
3537
#include "llvm/Support/Printable.h"
3638
#include "llvm/Support/raw_ostream.h"
39+
#include <utility>
3740

3841
namespace llvm {
3942

40-
struct LaneBitmask {
41-
// When changing the underlying type, change the format string as well.
42-
using Type = uint64_t;
43-
enum : unsigned { BitWidth = 8*sizeof(Type) };
44-
constexpr static const char *const FormatStr = "%016llX";
43+
struct LaneBitmask {
44+
static constexpr unsigned int BitWidth = 128;
4545

46-
constexpr LaneBitmask() = default;
47-
explicit constexpr LaneBitmask(Type V) : Mask(V) {}
48-
49-
constexpr bool operator== (LaneBitmask M) const { return Mask == M.Mask; }
50-
constexpr bool operator!= (LaneBitmask M) const { return Mask != M.Mask; }
51-
constexpr bool operator< (LaneBitmask M) const { return Mask < M.Mask; }
52-
constexpr bool none() const { return Mask == 0; }
53-
constexpr bool any() const { return Mask != 0; }
54-
constexpr bool all() const { return ~Mask == 0; }
55-
56-
constexpr LaneBitmask operator~() const {
57-
return LaneBitmask(~Mask);
58-
}
59-
constexpr LaneBitmask operator|(LaneBitmask M) const {
60-
return LaneBitmask(Mask | M.Mask);
61-
}
62-
constexpr LaneBitmask operator&(LaneBitmask M) const {
63-
return LaneBitmask(Mask & M.Mask);
64-
}
65-
LaneBitmask &operator|=(LaneBitmask M) {
66-
Mask |= M.Mask;
67-
return *this;
68-
}
69-
LaneBitmask &operator&=(LaneBitmask M) {
70-
Mask &= M.Mask;
71-
return *this;
46+
explicit LaneBitmask(APInt V) {
47+
switch (V.getBitWidth()) {
48+
case BitWidth:
49+
Mask[0] = V.getRawData()[0];
50+
Mask[1] = V.getRawData()[1];
51+
break;
52+
default:
53+
llvm_unreachable("Unsupported bitwidth");
7254
}
55+
}
56+
constexpr explicit LaneBitmask(uint64_t Lo = 0, uint64_t Hi = 0) : Mask{Lo, Hi} {}
7357

74-
constexpr Type getAsInteger() const { return Mask; }
58+
constexpr bool operator==(LaneBitmask M) const {
59+
return Mask[0] == M.Mask[0] && Mask[1] == M.Mask[1];
60+
}
61+
constexpr bool operator!=(LaneBitmask M) const {
62+
return Mask[0] != M.Mask[0] || Mask[1] != M.Mask[1];
63+
}
64+
constexpr bool operator<(LaneBitmask M) const {
65+
return Mask[1] < M.Mask[1] || Mask[0] < M.Mask[0];
66+
}
67+
constexpr bool none() const { return Mask[0] == 0 && Mask[1] == 0; }
68+
constexpr bool any() const { return Mask[0] != 0 || Mask[1] != 0; }
69+
constexpr bool all() const { return ~Mask[0] == 0 && ~Mask[1] == 0; }
7570

76-
unsigned getNumLanes() const { return llvm::popcount(Mask); }
77-
unsigned getHighestLane() const {
78-
return Log2_64(Mask);
79-
}
71+
constexpr LaneBitmask operator~() const { return LaneBitmask(~Mask[0], ~Mask[1]); }
72+
constexpr LaneBitmask operator|(LaneBitmask M) const {
73+
return LaneBitmask(Mask[0] | M.Mask[0], Mask[1] | M.Mask[1]);
74+
}
75+
constexpr LaneBitmask operator&(LaneBitmask M) const {
76+
return LaneBitmask(Mask[0] & M.Mask[0], Mask[1] & M.Mask[1]);
77+
}
78+
LaneBitmask &operator|=(LaneBitmask M) {
79+
Mask[0] |= M.Mask[0];
80+
Mask[1] |= M.Mask[1];
81+
return *this;
82+
}
83+
LaneBitmask &operator&=(LaneBitmask M) {
84+
Mask[0] &= M.Mask[0];
85+
Mask[1] &= M.Mask[1];
86+
return *this;
87+
}
8088

81-
static constexpr LaneBitmask getNone() { return LaneBitmask(0); }
82-
static constexpr LaneBitmask getAll() { return ~LaneBitmask(0); }
83-
static constexpr LaneBitmask getLane(unsigned Lane) {
84-
return LaneBitmask(Type(1) << Lane);
85-
}
89+
APInt getAsAPInt() const { return APInt(BitWidth, {Mask[0], Mask[1]}); }
90+
constexpr std::pair<uint64_t, uint64_t> getAsPair() const { return {Mask[0], Mask[1]}; }
8691

87-
private:
88-
Type Mask = 0;
89-
};
92+
unsigned getNumLanes() const {
93+
return Mask[1] ? llvm::popcount(Mask[1]) + llvm::popcount(Mask[0])
94+
: llvm::popcount(Mask[0]);
95+
}
96+
unsigned getHighestLane() const {
97+
return Mask[1] ? Log2_64(Mask[1]) + 64 : Log2_64(Mask[0]);
98+
}
9099

91-
/// Create Printable object to print LaneBitmasks on a \ref raw_ostream.
92-
inline Printable PrintLaneMask(LaneBitmask LaneMask) {
93-
return Printable([LaneMask](raw_ostream &OS) {
94-
OS << format(LaneBitmask::FormatStr, LaneMask.getAsInteger());
95-
});
100+
static constexpr LaneBitmask getNone() { return LaneBitmask(0, 0); }
101+
static constexpr LaneBitmask getAll() { return ~LaneBitmask(0, 0); }
102+
static constexpr LaneBitmask getLane(unsigned Lane) {
103+
return Lane >= 64 ? LaneBitmask(0, 1ULL << (Lane % 64))
104+
: LaneBitmask(1ULL << Lane, 0);
96105
}
97106

107+
private:
108+
uint64_t Mask[2];
109+
};
110+
111+
/// Create Printable object to print LaneBitmasks on a \ref raw_ostream.
112+
/// If \p FormatAsCLiterals is true, it will print the bitmask as
113+
/// a hexadecimal C literal with zero padding, or a list of such C literals if
114+
/// the value cannot be represented in 64 bits.
115+
/// For example (FormatAsCliterals == true)
116+
/// bitmask '1' => "0x0000000000000001"
117+
/// bitmask '1 << 64' => "0x0000000000000000,0x0000000000000001"
118+
/// (FormatAsCLiterals == false)
119+
/// bitmask '1' => "00000000000000000000000000000001"
120+
/// bitmask '1 << 64' => "00000000000000010000000000000000"
121+
inline Printable PrintLaneMask(LaneBitmask LaneMask,
122+
bool FormatAsCLiterals = false) {
123+
return Printable([LaneMask, FormatAsCLiterals](raw_ostream &OS) {
124+
SmallString<64> Buffer;
125+
APInt V = LaneMask.getAsAPInt();
126+
while (true) {
127+
unsigned Bitwidth = FormatAsCLiterals ? 64 : LaneBitmask::BitWidth;
128+
APInt VToPrint = V.trunc(Bitwidth);
129+
130+
Buffer.clear();
131+
VToPrint.toString(Buffer, 16, /*Signed=*/false,
132+
/*formatAsCLiteral=*/false);
133+
unsigned NumZeroesToPad =
134+
(VToPrint.countLeadingZeros() / 4) - VToPrint.isZero();
135+
OS << (FormatAsCLiterals ? "0x" : "") << std::string(NumZeroesToPad, '0')
136+
<< Buffer.str();
137+
V = V.lshr(Bitwidth);
138+
if (V.getActiveBits())
139+
OS << ",";
140+
else
141+
break;
142+
}
143+
});
144+
}
145+
98146
} // end namespace llvm
99147

100148
#endif // LLVM_MC_LANEBITMASK_H

llvm/lib/CodeGen/MIRParser/MIParser.cpp

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -870,17 +870,40 @@ bool MIParser::parseBasicBlockLiveins(MachineBasicBlock &MBB) {
870870
lex();
871871
LaneBitmask Mask = LaneBitmask::getAll();
872872
if (consumeIfPresent(MIToken::colon)) {
873-
// Parse lane mask.
874-
if (Token.isNot(MIToken::IntegerLiteral) &&
875-
Token.isNot(MIToken::HexLiteral))
876-
return error("expected a lane mask");
877-
static_assert(sizeof(LaneBitmask::Type) == sizeof(uint64_t),
878-
"Use correct get-function for lane mask");
879-
LaneBitmask::Type V;
880-
if (getUint64(V))
881-
return error("invalid lane mask value");
882-
Mask = LaneBitmask(V);
883-
lex();
873+
if (consumeIfPresent(MIToken::lparen)) {
874+
// We need to parse a list of literals
875+
SmallVector<uint64_t, 2> Literals;
876+
while (true) {
877+
if (Token.isNot(MIToken::HexLiteral))
878+
return error("expected a lane mask");
879+
APInt V;
880+
getHexUint(V);
881+
Literals.push_back(V.getZExtValue());
882+
// Lex past literal
883+
lex();
884+
if (Token.is(MIToken::rparen))
885+
break;
886+
else if (Token.isNot(MIToken::comma))
887+
return error("expected a comma");
888+
// Lex past comma
889+
lex();
890+
}
891+
// Lex past rparen
892+
lex();
893+
Mask = LaneBitmask(APInt(LaneBitmask::BitWidth, Literals));
894+
} else {
895+
// Parse lane mask.
896+
APInt V;
897+
if (Token.is(MIToken::IntegerLiteral)) {
898+
uint64_t UV;
899+
if (getUint64(UV))
900+
return error("invalid lane mask value");
901+
V = APInt(LaneBitmask::BitWidth, UV);
902+
} else if (getHexUint(V))
903+
return error("expected a lane mask");
904+
Mask = LaneBitmask(APInt(LaneBitmask::BitWidth, V.getZExtValue()));
905+
lex();
906+
}
884907
}
885908
MBB.addLiveIn(Reg, Mask);
886909
} while (consumeIfPresent(MIToken::comma));

llvm/lib/CodeGen/MIRPrinter.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -732,8 +732,14 @@ void MIPrinter::print(const MachineBasicBlock &MBB) {
732732
OS << ", ";
733733
First = false;
734734
OS << printReg(LI.PhysReg, &TRI);
735-
if (!LI.LaneMask.all())
736-
OS << ":0x" << PrintLaneMask(LI.LaneMask);
735+
if (!LI.LaneMask.all()) {
736+
OS << ":";
737+
if (LI.LaneMask.getAsAPInt().getActiveBits() <= 64)
738+
OS << PrintLaneMask(LI.LaneMask, /*FormatAsCLiterals=*/true);
739+
else
740+
OS << '(' << PrintLaneMask(LI.LaneMask, /*FormatAsCLiterals=*/true)
741+
<< ')';
742+
}
737743
}
738744
OS << "\n";
739745
HasLineAttributes = true;

llvm/lib/CodeGen/RDFRegisters.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -412,11 +412,11 @@ raw_ostream &operator<<(raw_ostream &OS, const PrintLaneMaskShort &P) {
412412
if (P.Mask.none())
413413
return OS << ":*none*";
414414

415-
LaneBitmask::Type Val = P.Mask.getAsInteger();
416-
if ((Val & 0xffff) == Val)
417-
return OS << ':' << format("%04llX", Val);
418-
if ((Val & 0xffffffff) == Val)
419-
return OS << ':' << format("%08llX", Val);
415+
APInt Val = P.Mask.getAsAPInt();
416+
if (Val.getActiveBits() <= 16)
417+
return OS << ':' << format("%04llX", Val.getZExtValue());
418+
if (Val.getActiveBits() <= 32)
419+
return OS << ':' << format("%08llX", Val.getZExtValue());
420420
return OS << ':' << PrintLaneMask(P.Mask);
421421
}
422422

llvm/lib/Target/AMDGPU/SIRegisterInfo.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,10 @@ class SIRegisterInfo final : public AMDGPUGenRegisterInfo {
377377
static unsigned getNumCoveredRegs(LaneBitmask LM) {
378378
// The assumption is that every lo16 subreg is an even bit and every hi16
379379
// is an adjacent odd bit or vice versa.
380-
uint64_t Mask = LM.getAsInteger();
380+
APInt MaskV = LM.getAsAPInt();
381+
assert(MaskV.getActiveBits() <= 64 &&
382+
"uint64_t is insufficient to represent lane bitmask operation");
383+
uint64_t Mask = MaskV.getZExtValue();
381384
uint64_t Even = Mask & 0xAAAAAAAAAAAAAAAAULL;
382385
Mask = (Even >> 1) | Mask;
383386
uint64_t Odd = Mask & 0x5555555555555555ULL;
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
2+
# RUN: llc -o - %s -mtriple=aarch64 -stop-before=greedy | FileCheck %s
3+
---
4+
name: test_parse_lanebitmask
5+
tracksRegLiveness: true
6+
liveins:
7+
- { reg: '$h0' }
8+
- { reg: '$s1' }
9+
body: |
10+
bb.0:
11+
liveins: $h0:0x0000000000000001, $s1:(0x0000000000000001,0x0000000000000000)
12+
; CHECK-LABEL: name: test_parse_lanebitmask
13+
; CHECK: liveins: $h0:0x0000000000000001, $s1:0x0000000000000001, $h0, $s1
14+
; CHECK-NEXT: {{ $}}
15+
; CHECK-NEXT: RET_ReallyLR
16+
RET_ReallyLR
17+
...
18+

llvm/unittests/CodeGen/MFCommon.inc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class BogusRegisterInfo : public TargetRegisterInfo {
2323
public:
2424
BogusRegisterInfo()
2525
: TargetRegisterInfo(nullptr, BogusRegisterClasses, BogusRegisterClasses,
26-
nullptr, nullptr, nullptr, LaneBitmask(~0u), nullptr,
26+
nullptr, nullptr, nullptr, LaneBitmask::getAll(), nullptr,
2727
nullptr) {
2828
InitMCRegisterInfo(nullptr, 0, 0, 0, nullptr, 0, nullptr, 0, nullptr,
2929
nullptr, nullptr, nullptr, nullptr, 0, nullptr);

llvm/unittests/MC/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ add_llvm_unittest(MCTests
1717
Disassembler.cpp
1818
DwarfLineTables.cpp
1919
DwarfLineTableHeaders.cpp
20+
LaneBitmaskTest.cpp
2021
MCInstPrinter.cpp
2122
StringTableBuilderTest.cpp
2223
TargetRegistry.cpp
2324
MCDisassemblerTest.cpp
2425
)
25-

0 commit comments

Comments
 (0)