[PATCH] D11070: WebAssembly: basic instructions todo, and basic register info.
JF Bastien
jfb at chromium.org
Thu Jul 9 16:17:52 PDT 2015
jfb added a comment.
Updated, PTAL.
================
Comment at: lib/Target/WebAssembly/Makefile:16
@@ -16,1 +15,3 @@
+BUILT_SOURCES = WebAssemblyGenRegisterInfo.inc WebAssemblyGenSubtargetInfo.inc \
+ WebAssemblyGenMCCodeEmitter.inc
----------------
Those are tabs (same as for other Makefiles), they just don't appear nicely in phab.
================
Comment at: lib/Target/WebAssembly/WebAssemblyInstrInfo.td:50
@@ +49,3 @@
+ * return the second operand
+ * conditional: basically ternary ?: operator
+*/
----------------
Good point, I removed them.
================
Comment at: lib/Target/WebAssembly/WebAssemblyRegisterInfo.cpp:49
@@ +48,3 @@
+ Reserved.set(WebAssembly::SP32);
+ Reserved.set(WebAssembly::SP64);
+ return Reserved;
----------------
Oops, I added FP later and forgot to add it here too. Changed to a range-based for loop too.
================
Comment at: lib/Target/WebAssembly/WebAssemblyRegisterInfo.cpp:61
@@ +60,3 @@
+WebAssemblyRegisterInfo::getFrameRegister(const MachineFunction &MF) const {
+ static const unsigned char Regs[2][2] = {
+ /* !isArch64Bit isArch64Bit */
----------------
Imagine all the bits we're wasting! ;-)
================
Comment at: lib/Target/WebAssembly/WebAssemblyRegisterInfo.cpp:65
@@ +64,3 @@
+ /* hasFP */ {WebAssembly::FP32, WebAssembly::FP64}};
+ const TargetFrameLowering *TFI = MF.getSubtarget().getFrameLowering();
+ return Regs[TFI->hasFP(MF)][TT.isArch64Bit()];
----------------
I'll send a separate PR to do the same in other backends.
================
Comment at: lib/Target/WebAssembly/WebAssemblyRegisterInfo.td:30
@@ +29,3 @@
+// WebAssembly supports mixed 32-bit and 64-bit heaps in the same application,
+// which requires separate width FP and SP.
+def FP32 : WebAssemblyReg<"%FP32">;
----------------
Are you agreeing, or suggesting something?
================
Comment at: lib/Target/WebAssembly/WebAssemblyRegisterInfo.td:43
@@ +42,3 @@
+ def D#i : WebAssemblyReg<"%d."#i>; // f64
+}
+
----------------
I'll add a todo instead to try it out later, if needed? I'd rather follow what others have done, and then try to change it, seems like we'll have less WTF moments that way.
http://reviews.llvm.org/D11070
More information about the llvm-commits
mailing list