[PATCH] D43706: [WebAssembly] Add except_ref as a first-class type

Derek Schuff via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 7 16:51:34 PST 2018


dschuff added a comment.

Everything else seems pretty straightforward.



================
Comment at: include/llvm/CodeGen/MachineValueType.h:751
       case nxv32i64: return 2048;
+      case ExceptRef: return 0; // opaque type
       }
----------------
aheejin wrote:
> dschuff wrote:
> > This seems fishy. What would a caller actually do with a 0-size type? Do we expect this to ever be called? Should it be an unreachable instead?
> Yes, it is called [[ https://github.com/llvm-mirror/llvm/blob/587a2e31eca58dbc0a7535c70aa3bbbb5cfe733c/utils/TableGen/CodeGenRegisters.cpp#L772-L773 | here ]], while tablegen generates tables. We need this line, because if it's an unreachable, it will crash the LLVM build while running tablegen.
> 
> Also we have one more 0 in this CL in this line in `lib/Target/WebAssembly/WebAssemblyRegisterInfo.td`:
> ```
> def EXCEPT_REF : WebAssemblyRegClass<[ExceptRef], 0, (add EXCEPT_REF_0)>;
> ```
> 
> These two pieces of information is used by tablegen to generate this array in `$(LLVM_ROOT)/build/lib/Target/WebAssembly/WebAssemblyGenRegisterInfo.inc`:
> ```
> static const TargetRegisterInfo::RegClassInfo RegClassInfos[] = {
>   // Mode = 0 (Default)
>   { 0, 0, 0, VTLists+13 },    // EXCEPT_REF
>   { 32, 32, 32, VTLists+0 },    // I32
>   { 32, 32, 32, VTLists+4 },    // F32
>   { 64, 64, 64, VTLists+2 },    // I64
>   { 64, 64, 64, VTLists+6 },    // F64
>   { 128, 128, 128, VTLists+8 },    // V128
> };
> ```
> 
> There is a line for each register class in this array. There are three zeros in the first line for `EXCEPT_REF` register class. First two zeros come from `case ExceptRef: return 0;` here and the third zero is from that `def EXCEPT_REF : ...` line from `lib/Target/WebAssembly/WebAssemblyRegisterInfo.td`. Each line makes up an instance of [[ https://github.com/llvm-mirror/llvm/blob/587a2e31eca58dbc0a7535c70aa3bbbb5cfe733c/include/llvm/CodeGen/TargetRegisterInfo.h#L224-L227 | `TargetRegisterInfo::RegClassInfo` ]].
> 
> So these 0s become `RegSize`, `SpillSize`, and `SpillAlignment` member variables in [[ https://github.com/llvm-mirror/llvm/blob/587a2e31eca58dbc0a7535c70aa3bbbb5cfe733c/include/llvm/CodeGen/TargetRegisterInfo.h#L224-L227 | `TargetRegisterInfo::RegClassInfo` ]] class. But in wasm we don't use this kind of size information anywhere because we don't use physical registers or run register allocation pass.
Hm yeah I guess that makes sense and we are already weird when it comes to spilling (we don't). It seems like the other common case it's used for is getting sub/super registers (which we also don't need, but there's target-independent code) and debug info. Hopefully that won't be an issue since they can't be written to memory. Seems like 0 should be ok for now since it's more likely to expose any issues quickly compared to picking a number.  But regardless of what we pick here we'll probably be revisiting this issue again.


Repository:
  rL LLVM

https://reviews.llvm.org/D43706





More information about the llvm-commits mailing list