Skip to content

Commit c3afe10

Browse files
committed
Addressed the reviews related to dewscription & documentation of the new operand.
1 parent f838e74 commit c3afe10

File tree

12 files changed

+42
-34
lines changed

12 files changed

+42
-34
lines changed

llvm/docs/MIRLangRef.rst

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -807,23 +807,23 @@ For an int eq predicate ``ICMP_EQ``, the syntax is:
807807
808808
%2:gpr(s32) = G_ICMP intpred(eq), %0, %1
809809
810-
Lanemask Operands
810+
LaneMask Operands
811811
^^^^^^^^^^^^^^^^^^
812812

813-
A Lanemask operand is a 64-bit unsigned value that can store lane information
814-
for a register operand in the instruction. It can be used as many times as needed
815-
in an instruction, with one or more register operands associated with it. While
816-
the active bits represent the live subregister (in virtual registers) or regUnits
817-
(in physical registers), the remaining bits can represent the UNDEF part of it.
813+
A LaneMask operand contains a LaneBitmask struct representing the covering of a
814+
register with sub-registers. [That's what it says in LaneBitmask.h!] Instructions
815+
typically associate a LaneMask operand with one or more Register operands, and
816+
use it to represent sub-register granularity information like liveness for those
817+
associated Register operands.
818818

819819

820-
For example, the COPY_LANEMASK instruction uses this operand to copy only active
820+
For example, the COPY_LANEMASK instruction uses this operand to copy only active
821821
lanes (of the source register) in the mask. The syntax for it would look like:
822822

823823
.. code-block:: text
824824
825825
$vgpr1 = COPY_LANEMASK $vgpr0, lanemask(0x00000000000000C0)
826-
826+
827827
.. TODO: Describe the parsers default behaviour when optional YAML attributes
828828
are missing.
829829
.. TODO: Describe the syntax for virtual register YAML definitions.

llvm/include/llvm/CodeGen/MachineInstr.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1432,7 +1432,7 @@ class MachineInstr
14321432
return getOpcode() == TargetOpcode::COPY;
14331433
}
14341434

1435-
bool isCopyLanemask() const {
1435+
bool isCopyLaneMask() const {
14361436
return getOpcode() == TargetOpcode::COPY_LANEMASK;
14371437
}
14381438

llvm/include/llvm/Support/TargetOpcodes.def

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,9 @@ HANDLE_TARGET_OPCODE(REG_SEQUENCE)
114114
/// used to copy between subregisters of virtual registers.
115115
HANDLE_TARGET_OPCODE(COPY)
116116

117-
/// COPY_LANEMASK - Target-independent register copy for active mask in
118-
/// register as represented by the lanemask. This instruction does not
117+
/// COPY_LANEMASK - Target-independent partial register copy. The laneMask
118+
/// operand indicates which parts of the source register are copied to the
119+
/// destination. Other parts of the destination are undefined. It does not
119120
/// support copy between subregisters of virtual registers.
120121
HANDLE_TARGET_OPCODE(COPY_LANEMASK)
121122

llvm/lib/CodeGen/MIRParser/MIParser.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2881,22 +2881,22 @@ bool MIParser::parseLaneMaskOperand(MachineOperand &Dest) {
28812881

28822882
lex();
28832883
if (expectAndConsume(MIToken::lparen))
2884-
return error("lanemask should begin with '('.");
2884+
return true;
28852885

28862886
LaneBitmask LaneMask = LaneBitmask::getAll();
28872887
// Parse lanemask.
28882888
if (Token.isNot(MIToken::IntegerLiteral) && Token.isNot(MIToken::HexLiteral))
2889-
return error("expected a valid lane mask value.");
2889+
return error("expected a valid lane mask value");
28902890
static_assert(sizeof(LaneBitmask::Type) == sizeof(uint64_t),
28912891
"Use correct get-function for lane mask.");
28922892
LaneBitmask::Type V;
28932893
if (getUint64(V))
2894-
return error("invalid lanemask value");
2894+
return true;
28952895
LaneMask = LaneBitmask(V);
28962896
lex();
28972897

28982898
if (expectAndConsume(MIToken::rparen))
2899-
return error("lanemask should be terminated by ')'.");
2899+
return true;
29002900

29012901
Dest = MachineOperand::CreateLaneMask(LaneMask);
29022902
return false;

llvm/lib/CodeGen/MachineVerifier.cpp

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2434,7 +2434,7 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
24342434
const MachineOperand &LaneMaskOp = MI->getOperand(2);
24352435
const Register SrcReg = SrcOp.getReg();
24362436
const LaneBitmask LaneMask = LaneMaskOp.getLaneMask();
2437-
LaneBitmask SrcMaxLanemask = LaneBitmask::getAll();
2437+
LaneBitmask SrcMaxLaneMask = LaneBitmask::getAll();
24382438

24392439
if (DstOp.getSubReg())
24402440
report("COPY_LANEMASK must not use a subregister index", &DstOp, 0);
@@ -2448,15 +2448,22 @@ void MachineVerifier::visitMachineInstrBefore(const MachineInstr *MI) {
24482448
if (SrcReg.isPhysical()) {
24492449
const TargetRegisterClass *SrcRC = TRI->getMinimalPhysRegClass(SrcReg);
24502450
if (SrcRC)
2451-
SrcMaxLanemask = SrcRC->getLaneMask();
2451+
SrcMaxLaneMask = SrcRC->getLaneMask();
24522452
} else {
2453-
SrcMaxLanemask = MRI->getMaxLaneMaskForVReg(SrcReg);
2453+
SrcMaxLaneMask = MRI->getMaxLaneMaskForVReg(SrcReg);
24542454
}
24552455

2456-
// If LaneMask is equal to OR greater than the SrcMaxLanemask, it
2457-
// impliess COPY_LANEMASK is trying to copy all lanes.
2458-
if (SrcMaxLanemask <= LaneMask)
2459-
report("COPY_LANEMASK cannot read all lanes", MI);
2456+
// COPY_LANEMASK should be used only for partial copy. For full
2457+
// copy, one should strictly use the COPY instruction.
2458+
if (SrcMaxLaneMask == LaneMask)
2459+
report("COPY_LANEMASK cannot be used to do full copy", MI);
2460+
2461+
// If LaneMask is equal to OR greater than the SrcMaxLaneMask, it
2462+
// implies COPY_LANEMASK is attempting to read from the lanes that
2463+
// don't exists in the source register.
2464+
if (SrcMaxLaneMask < LaneMask)
2465+
report("COPY_LANEMASK attempts to read from the lanes that "
2466+
"don't exist in the source register", MI);
24602467

24612468
break;
24622469
}

llvm/test/CodeGen/MIR/AMDGPU/parse-lanemask-operand-invalid-0.mir

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ body: |
77
bb.0:
88
liveins: $vgpr0
99
10-
; CHECK: [[@LINE+1]]:47: lanemask should be terminated by ')'.
10+
; CHECK: [[@LINE+1]]:47: expected ')'
1111
$vgpr1 = COPY_LANEMASK $vgpr0, lanemask(16
1212
S_ENDPGM 0
1313
...

llvm/test/CodeGen/MIR/AMDGPU/parse-lanemask-operand-invalid-1.mir

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ body: |
77
bb.0:
88
liveins: $vgpr0
99
10-
; CHECK: [[@LINE+1]]:45: lanemask should begin with '('.
10+
; CHECK: [[@LINE+1]]:45: expected '('
1111
$vgpr1 = COPY_LANEMASK $vgpr0, lanemask 14)
1212
S_ENDPGM 0
1313
...

llvm/test/CodeGen/MIR/AMDGPU/parse-lanemask-operand-invalid-2.mir

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ body: |
77
bb.0:
88
liveins: $vgpr0
99
10-
; CHECK: [[@LINE+1]]:45: expected a valid lane mask value.
10+
; CHECK: [[@LINE+1]]:45: expected a valid lane mask value
1111
$vgpr1 = COPY_LANEMASK $vgpr0, lanemask(undef)
1212
S_ENDPGM 0
1313
...

llvm/test/CodeGen/MIR/AMDGPU/parse-lanemask-operand-invalid-3.mir

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ body: |
77
bb.0:
88
liveins: $vgpr0
99
10-
; CHECK: [[@LINE+1]]:45: expected a valid lane mask value.
10+
; CHECK: [[@LINE+1]]:45: expected a valid lane mask value
1111
$vgpr1 = COPY_LANEMASK $vgpr0, lanemask()
1212
S_ENDPGM 0
1313
...

llvm/test/CodeGen/RISCV/GlobalISel/legalizer-info-validation.mir

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@
7777
# DEBUG-NEXT: .. imm index coverage check SKIPPED: user-defined predicate detected
7878
#
7979
# DEBUG-NEXT: G_ABDU (opcode 67): 1 type index, 0 imm indices
80-
# DEBUG-NEXT: .. opcode 67 is aliased to 66
80+
# DEBUG-NEXT: .. opcode 67 is aliased to 66
8181
# DEBUG-NEXT: .. type index coverage check SKIPPED: user-defined predicate detected
8282
# DEBUG-NEXT: .. imm index coverage check SKIPPED: user-defined predicate detected
8383
#

0 commit comments

Comments
 (0)