[WebAssembly] tablegen: distinguish float/int immediate operands.

[aardappel]

Summary

Before, they were one category of operands which could cause
crashes in non-sensical combinations, e.g. "f32.const symbol".
Now these are forced to be an error.

[dschuff]

Wow, that was simple.
Is there a "bad" asm parser test we can add now to test this?

[aardappel]

We don't have "error" tests for any other illegal operand possibilities, but sure, added a test.

[aardappel]

added test

[aheejin]

lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

92

Isn't this supposed to include both integer and float immediates? (It is an inherited one, so)

[aardappel]

lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

92

That really depends on how you define it. "Imm" is a class name driven by the tablegen def, that we happened to use for both int and float, which was problematic, since our emitter assumed they were separate. With this change I force them to be separate in both the def and the code.

[aheejin]

lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

92

I see, thanks.

lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

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 isImm() const override { return Kind == Integer || Kind == Symbol; }
+  bool isFPImm() const { return Kind == Float; }
   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");
     return 0;
   }
void addRegOperands(MCInst &, unsigned) const {
     // Required by the assembly matcher.
     llvm_unreachable("Assembly matcher creates register operands");
   }
 
   void addImmOperands(MCInst &Inst, unsigned N) const {
     assert(N == 1 && "Invalid number of operands!");
     if (Kind == Integer)
       Inst.addOperand(MCOperand::createImm(Int.Val));
-    else if (Kind == Float)
-      Inst.addOperand(MCOperand::createFPImm(Flt.Val));
     else if (Kind == Symbol)
       Inst.addOperand(MCOperand::createExpr(Sym.Exp));
     else
-      llvm_unreachable("Should be immediate or symbol!");
+      llvm_unreachable("Should be integer immediate or symbol!");
+  }
+
+  void addFPImmOperands(MCInst &Inst, unsigned N) const {
+    assert(N == 1 && "Invalid number of operands!");
+    if (Kind == Float)
+      Inst.addOperand(MCOperand::createFPImm(Flt.Val));
+    else
+      llvm_unreachable("Should be float immediate!");
   }
 
   void addBrListOperands(MCInst &Inst, unsigned N) const {
     assert(N == 1 && isBrList() && "Invalid BrList!");
     for (auto Br : BrL.List)
       Inst.addOperand(MCOperand::createImm(Br));
   }
 

lib/Target/WebAssembly/WebAssemblyInstrInfo.td

def WebAssemblywrapperPIC  : SDNode<"WebAssemblyISD::WrapperPIC",
                                      SDT_WebAssemblyWrapperPIC>;
 def WebAssemblythrow : SDNode<"WebAssemblyISD::THROW", SDT_WebAssemblyThrow,
                               [SDNPHasChain, SDNPVariadic]>;
 
 //===----------------------------------------------------------------------===//
 // WebAssembly-specific Operands.
 //===----------------------------------------------------------------------===//
 
+// Default Operand has AsmOperandClass "Imm" which is for integers (and
+// symbols), so specialize one for floats:
+def FPImmAsmOperand : AsmOperandClass {
+  let Name = "FPImm";
+  let PredicateMethod = "isFPImm";
+}
+
+class FPOperand<ValueType ty> : Operand<ty> {
+  AsmOperandClass ParserMatchClass = FPImmAsmOperand;
+}
+
 let OperandNamespace = "WebAssembly" in {
 
 let OperandType = "OPERAND_BASIC_BLOCK" in
 def bb_op : Operand<OtherVT>;
 
 let OperandType = "OPERAND_LOCAL" in
 def local_op : Operand<i32>;
 
 let OperandType = "OPERAND_GLOBAL" in
 def global_op : Operand<i32>;
 
 let OperandType = "OPERAND_I32IMM" in
 def i32imm_op : Operand<i32>;
 
 let OperandType = "OPERAND_I64IMM" in
 def i64imm_op : Operand<i64>;
 
 let OperandType = "OPERAND_F32IMM" in
-def f32imm_op : Operand<f32>;
+def f32imm_op : FPOperand<f32>;
 
 let OperandType = "OPERAND_F64IMM" in
-def f64imm_op : Operand<f64>;
+def f64imm_op : FPOperand<f64>;
 
 let OperandType = "OPERAND_VEC_I8IMM" in
 def vec_i8imm_op : Operand<i32>;
 
 let OperandType = "OPERAND_VEC_I16IMM" in
 def vec_i16imm_op : Operand<i32>;
 
 let OperandType = "OPERAND_VEC_I32IMM" in

test/MC/WebAssembly/basic-assembly-errors.s

 # RUN: not llvm-mc -triple=wasm32-unknown-unknown -mattr=+simd128,+nontrapping-fptoint,+exception-handling < %s 2>&1 | FileCheck %s
 
+# CHECK: invalid operand for instruction
+# (must be 0.0 or similar)
+    f32.const 0
+
 # CHECK: End of block construct with no start: end_try
     end_try
 test0:
     .functype   test0 () -> ()
 # CHECK: Block construct type mismatch, expected: end_function, instead got: end_loop
     end_loop
     block
 # CHECK: Block construct type mismatch, expected: end_block, instead got: end_if