Skip to content

Commit

Permalink
[WebAssembly] Fix assembler parsing of br_table.
Browse files Browse the repository at this point in the history 
Summary:
We use `variable_ops` in the tablegen defs to denote the list of
branch targets in `br_table`, but unlike other uses of `variable_ops`
(e.g. call) the these branch targets need to actually be encoded in the
instruction. The existing tables for `variable_ops` cause not operands
to be accepted by the assembly matcher.

Following the example of ARM:
https://github.com/llvm-mirror/llvm/blob/2cc0a7da876c1d8c32775b0119e1e15aaa759b9e/lib/Target/ARM/ARMInstrInfo.td#L550-L555
we introduce a new operand type to capture this list, and we use the
same {} syntax as ARM as well to differentiate them from regular
integer operands.

Also removed definition and use of TSFlags in tablegen defs, since
`br_table` now has a non-variable_ops immediate operand, so the
previous logic of only the variable_ops arguments being labels didn't
make sense anymore.

Reviewers: dschuff, aheejin, sunfish

Subscribers: javed.absar, sbc100, jgravelle-google, kristof.beyls, llvm-commits

Differential Revision: https://reviews.llvm.org/D55401

llvm-svn: 349405
  • Loading branch information
@aardappel
aardappel committed Dec 17, 2018
1 parent 8c9d772 commit d3c544a
Show file tree
Hide file tree
Showing 8 changed files with 105 additions and 62 deletions.
39 changes: 38 additions & 1 deletion llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
View file
Expand Up @@ -38,7 +38,7 @@ namespace {
/// WebAssemblyOperand - Instances of this class represent the operands in a
/// parsed WASM machine instruction.
struct WebAssemblyOperand : public MCParsedAsmOperand {
enum KindTy { Token, Integer, Float, Symbol } Kind;
enum KindTy { Token, Integer, Float, Symbol, BrList } Kind;

SMLoc StartLoc, EndLoc;

Expand All @@ -58,11 +58,16 @@ struct WebAssemblyOperand : public MCParsedAsmOperand {
const MCExpr *Exp;
};

struct BrLOp {
std::vector<unsigned> List;
};

union {
struct TokOp Tok;
struct IntOp Int;
struct FltOp Flt;
struct SymOp Sym;
struct BrLOp BrL;
};

WebAssemblyOperand(KindTy K, SMLoc Start, SMLoc End, TokOp T)
Expand All @@ -73,13 +78,21 @@ struct WebAssemblyOperand : public MCParsedAsmOperand {
: Kind(K), StartLoc(Start), EndLoc(End), Flt(F) {}
WebAssemblyOperand(KindTy K, SMLoc Start, SMLoc End, SymOp S)
: Kind(K), StartLoc(Start), EndLoc(End), Sym(S) {}
WebAssemblyOperand(KindTy K, SMLoc Start, SMLoc End)
: Kind(K), StartLoc(Start), EndLoc(End), BrL() {}

~WebAssemblyOperand() {
if (isBrList())
BrL.~BrLOp();
}

bool isToken() const override { return Kind == Token; }
bool isImm() const override {
return Kind == Integer || Kind == Float || Kind == Symbol;
}
bool isMem() const override { return false; }
bool isReg() const override { return false; }
bool isBrList() const { return Kind == BrList; }

unsigned getReg() const override {
llvm_unreachable("Assembly inspects a register operand");
Expand Down Expand Up @@ -111,6 +124,12 @@ struct WebAssemblyOperand : public MCParsedAsmOperand {
llvm_unreachable("Should be immediate or symbol!");
}

void addBrListOperands(MCInst &Inst, unsigned N) const {
assert(N == 1 && isBrList() && "Invalid BrList!");
for (auto Br : BrL.List)
Inst.addOperand(MCOperand::createImm(Br));
}

void print(raw_ostream &OS) const override {
switch (Kind) {
case Token:
Expand All @@ -125,6 +144,9 @@ struct WebAssemblyOperand : public MCParsedAsmOperand {
case Symbol:
OS << "Sym:" << Sym.Exp;
break;
case BrList:
OS << "BrList:" << BrL.List.size();
break;
}
}
};
Expand Down Expand Up @@ -340,6 +362,21 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
Parser.Lex();
break;
}
case AsmToken::LCurly: {
Parser.Lex();
auto Op = make_unique<WebAssemblyOperand>(
WebAssemblyOperand::BrList, Tok.getLoc(), Tok.getEndLoc());
if (!Lexer.is(AsmToken::RCurly))
for (;;) {
Op->BrL.List.push_back(Lexer.getTok().getIntVal());
expect(AsmToken::Integer, "integer");
if (!isNext(AsmToken::Comma))
break;
}
expect(AsmToken::RCurly, "}");
Operands.push_back(std::move(Op));
break;
}
default:
return error("Unexpected token in operand: ", Tok);
}
Expand Down
51 changes: 24 additions & 27 deletions llvm/lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.cpp
View file
Expand Up @@ -134,11 +134,19 @@ void WebAssemblyInstPrinter::printInst(const MCInst *MI, raw_ostream &OS,
unsigned NumFixedOperands = Desc.NumOperands;
SmallSet<uint64_t, 8> Printed;
for (unsigned i = 0, e = MI->getNumOperands(); i < e; ++i) {
if (!(i < NumFixedOperands
? (Desc.OpInfo[i].OperandType ==
WebAssembly::OPERAND_BASIC_BLOCK)
: (Desc.TSFlags & WebAssemblyII::VariableOpImmediateIsLabel)))
continue;
// See if this operand denotes a basic block target.
if (i < NumFixedOperands) {
// A non-variable_ops operand, check its type.
if (Desc.OpInfo[i].OperandType != WebAssembly::OPERAND_BASIC_BLOCK)
continue;
} else {
// A variable_ops operand, which currently can be immediates (used in
// br_table) which are basic block targets, or for call instructions
// when using -wasm-keep-registers (in which case they are registers,
// and should not be processed).
if (!MI->getOperand(i).isImm())
continue;
}
uint64_t Depth = MI->getOperand(i).getImm();
if (!Printed.insert(Depth).second)
continue;
Expand Down Expand Up @@ -194,9 +202,6 @@ void WebAssemblyInstPrinter::printOperand(const MCInst *MI, unsigned OpNo,
raw_ostream &O) {
const MCOperand &Op = MI->getOperand(OpNo);
if (Op.isReg()) {
assert((OpNo < MII.get(MI->getOpcode()).getNumOperands() ||
MII.get(MI->getOpcode()).TSFlags == 0) &&
"WebAssembly variable_ops register ops don't use TSFlags");
unsigned WAReg = Op.getReg();
if (int(WAReg) >= 0)
printRegName(O, WAReg);
Expand All @@ -210,23 +215,9 @@ void WebAssemblyInstPrinter::printOperand(const MCInst *MI, unsigned OpNo,
if (OpNo < MII.get(MI->getOpcode()).getNumDefs())
O << '=';
} else if (Op.isImm()) {
const MCInstrDesc &Desc = MII.get(MI->getOpcode());
assert((OpNo < Desc.getNumOperands() ||
(Desc.TSFlags & WebAssemblyII::VariableOpIsImmediate)) &&
"WebAssemblyII::VariableOpIsImmediate should be set for "
"variable_ops immediate ops");
(void)Desc;
// TODO: (MII.get(MI->getOpcode()).TSFlags &
// WebAssemblyII::VariableOpImmediateIsLabel)
// can tell us whether this is an immediate referencing a label in the
// control flow stack, and it may be nice to pretty-print.
O << Op.getImm();
} else if (Op.isFPImm()) {
const MCInstrDesc &Desc = MII.get(MI->getOpcode());
assert(OpNo < Desc.getNumOperands() &&
"Unexpected floating-point immediate as a non-fixed operand");
assert(Desc.TSFlags == 0 &&
"WebAssembly variable_ops floating point ops don't use TSFlags");
const MCOperandInfo &Info = Desc.OpInfo[OpNo];
if (Info.OperandType == WebAssembly::OPERAND_F32IMM) {
// TODO: MC converts all floating point immediate operands to double.
Expand All @@ -237,16 +228,22 @@ void WebAssemblyInstPrinter::printOperand(const MCInst *MI, unsigned OpNo,
O << ::toString(APFloat(Op.getFPImm()));
}
} else {
assert((OpNo < MII.get(MI->getOpcode()).getNumOperands() ||
(MII.get(MI->getOpcode()).TSFlags &
WebAssemblyII::VariableOpIsImmediate)) &&
"WebAssemblyII::VariableOpIsImmediate should be set for "
"variable_ops expr ops");
assert(Op.isExpr() && "unknown operand kind in printOperand");
Op.getExpr()->print(O, &MAI);
}
}

void WebAssemblyInstPrinter::printBrList(const MCInst *MI, unsigned OpNo,
raw_ostream &O) {
O << "{";
for (unsigned I = OpNo, E = MI->getNumOperands(); I != E; ++I) {
if (I != OpNo)
O << ", ";
O << MI->getOperand(I).getImm();
}
O << "}";
}

void WebAssemblyInstPrinter::printWebAssemblyP2AlignOperand(const MCInst *MI,
unsigned OpNo,
raw_ostream &O) {
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Target/WebAssembly/InstPrinter/WebAssemblyInstPrinter.h
View file
Expand Up @@ -43,6 +43,7 @@ class WebAssemblyInstPrinter final : public MCInstPrinter {

// Used by tblegen code.
void printOperand(const MCInst *MI, unsigned OpNo, raw_ostream &O);
void printBrList(const MCInst *MI, unsigned OpNo, raw_ostream &O);
void printWebAssemblyP2AlignOperand(const MCInst *MI, unsigned OpNo,
raw_ostream &O);
void printWebAssemblySignatureOperand(const MCInst *MI, unsigned OpNo,
Expand Down
8 changes: 0 additions & 8 deletions llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp
View file
Expand Up @@ -89,8 +89,6 @@ void WebAssemblyMCCodeEmitter::encodeInstruction(

} else if (MO.isImm()) {
if (i < Desc.getNumOperands()) {
assert(Desc.TSFlags == 0 &&
"WebAssembly non-variable_ops don't use TSFlags");
const MCOperandInfo &Info = Desc.OpInfo[i];
LLVM_DEBUG(dbgs() << "Encoding immediate: type="
<< int(Info.OperandType) << "\n");
Expand Down Expand Up @@ -125,16 +123,10 @@ void WebAssemblyMCCodeEmitter::encodeInstruction(
encodeULEB128(uint64_t(MO.getImm()), OS);
}
} else {
assert(Desc.TSFlags == (WebAssemblyII::VariableOpIsImmediate |
WebAssemblyII::VariableOpImmediateIsLabel));
encodeULEB128(uint64_t(MO.getImm()), OS);
}

} else if (MO.isFPImm()) {
assert(i < Desc.getNumOperands() &&
"Unexpected floating-point immediate as a non-fixed operand");
assert(Desc.TSFlags == 0 &&
"WebAssembly variable_ops floating point ops don't use TSFlags");
const MCOperandInfo &Info = Desc.OpInfo[i];
if (Info.OperandType == WebAssembly::OPERAND_F32IMM) {
// TODO: MC converts all floating point immediate operands to double.
Expand Down
8 changes: 0 additions & 8 deletions llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h
View file
Expand Up @@ -83,14 +83,6 @@ enum OperandType {
} // end namespace WebAssembly

namespace WebAssemblyII {
enum {
// For variadic instructions, this flag indicates whether an operand
// in the variable_ops range is an immediate value.
VariableOpIsImmediate = (1 << 0),
// For immediate values in the variable_ops range, this flag indicates
// whether the value represents a control-flow label.
VariableOpImmediateIsLabel = (1 << 1)
};

/// Target Operand Flag enum.
enum TOF {
Expand Down
28 changes: 12 additions & 16 deletions llvm/lib/Target/WebAssembly/WebAssemblyInstrControl.td
View file
Expand Up @@ -33,41 +33,37 @@ def : Pat<(brcond (i32 (setne I32:$cond, 0)), bb:$dst),
def : Pat<(brcond (i32 (seteq I32:$cond, 0)), bb:$dst),
(BR_UNLESS bb_op:$dst, I32:$cond)>;

// A list of branch targets enclosed in {} and separated by comma.
// Used by br_table only.
def BrListAsmOperand : AsmOperandClass { let Name = "BrList"; }
def brlist : Operand<i32> {
let ParserMatchClass = BrListAsmOperand;
let PrintMethod = "printBrList";
}

// TODO: SelectionDAG's lowering insists on using a pointer as the index for
// jump tables, so in practice we don't ever use BR_TABLE_I64 in wasm32 mode
// currently.
// Set TSFlags{0} to 1 to indicate that the variable_ops are immediates.
// Set TSFlags{1} to 1 to indicate that the immediates represent labels.
// FIXME: this can't inherit from I<> since there is no way to inherit from a
// multiclass and still have the let statements.
let isTerminator = 1, hasCtrlDep = 1, isBarrier = 1 in {
let isCodeGenOnly = 1 in
def BR_TABLE_I32 : NI<(outs), (ins I32:$index, variable_ops),
[(WebAssemblybr_table I32:$index)], "false",
"br_table \t$index", 0x0e> {
let TSFlags{0} = 1;
let TSFlags{1} = 1;
}
let BaseName = "BR_TABLE_I32" in
def BR_TABLE_I32_S : NI<(outs), (ins variable_ops),
[], "true",
"br_table \t", 0x0e> {
let TSFlags{0} = 1;
let TSFlags{1} = 1;
def BR_TABLE_I32_S : NI<(outs), (ins brlist:$brl), [], "true",
"br_table \t$brl", 0x0e> {
}
let isCodeGenOnly = 1 in
def BR_TABLE_I64 : NI<(outs), (ins I64:$index, variable_ops),
[(WebAssemblybr_table I64:$index)], "false",
"br_table \t$index"> {
let TSFlags{0} = 1;
let TSFlags{1} = 1;
}
let BaseName = "BR_TABLE_I64" in
def BR_TABLE_I64_S : NI<(outs), (ins variable_ops),
[], "true",
"br_table \t"> {
let TSFlags{0} = 1;
let TSFlags{1} = 1;
def BR_TABLE_I64_S : NI<(outs), (ins brlist:$brl), [], "true",
"br_table \t$brl"> {
}
} // isTerminator = 1, hasCtrlDep = 1, isBarrier = 1

Expand Down
3 changes: 1 addition & 2 deletions llvm/test/CodeGen/WebAssembly/stack-insts.ll
View file
Expand Up @@ -8,8 +8,7 @@ declare void @foo1()

; Tests if br_table is printed correctly with a tab.
; CHECK-LABEL: test0:
; CHECK-NOT: br_table0, 1, 0, 1, 0
; CHECK: br_table 0, 1, 0, 1, 0
; CHECK: br_table {0, 1, 0, 1, 0}
define void @test0(i32 %n) {
entry:
switch i32 %n, label %sw.epilog [
Expand Down
29 changes: 29 additions & 0 deletions llvm/test/MC/WebAssembly/basic-assembly.s
View file
Expand Up @@ -45,6 +45,20 @@ test0:
end_block # label0:
get_local 4
get_local 5
block
block
block
block
br_table {0, 1, 2} # 2 entries, default
end_block # first entry jumps here.
i32.const 1
br 2
end_block # second entry jumps here.
i32.const 2
br 1
end_block # default jumps here.
i32.const 3
end_block # "switch" exit.
f32x4.add
# Test correct parsing of instructions with / and : in them:
# TODO: enable once instruction has been added.
Expand Down Expand Up @@ -100,6 +114,21 @@ test0:
# CHECK-NEXT: end_block # label0:
# CHECK-NEXT: get_local 4
# CHECK-NEXT: get_local 5
# CHECK-NEXT: block
# CHECK-NEXT: block
# CHECK-NEXT: block
# CHECK-NEXT: block
# CHECK-NEXT: br_table {0, 1, 2} # 1: down to label4
# CHECK-NEXT: # 2: down to label3
# CHECK-NEXT: end_block # label5:
# CHECK-NEXT: i32.const 1
# CHECK-NEXT: br 2 # 2: down to label2
# CHECK-NEXT: end_block # label4:
# CHECK-NEXT: i32.const 2
# CHECK-NEXT: br 1 # 1: down to label2
# CHECK-NEXT: end_block # label3:
# CHECK-NEXT: i32.const 3
# CHECK-NEXT: end_block # label2:
# CHECK-NEXT: f32x4.add
# CHECK-NEXT: i32.trunc_s/f32
# CHECK-NEXT: try
Expand Down

0 comments on commit d3c544a

Please sign in to comment.