[WebAssembly] More uses of uint8_t for single byte values

[aheejin]

Summary

It looks like this was missing from D43921.

[aheejin]

This broke test/MC/WebAssembly/global-ctor-dtor.ll in the waterfall. I checked the output but don't have any idea why it's causing differences in relocation and function information (I'm not familiar with the linker). Do you know why?

[ncw]

I saw the build failures too, when I was trying to land some patches today. I feel I can't land until the tests pass, even if the failures weren't caused by me.

This commit is causing the function body to change from:

from: 024041818080800041004180808080001081808080000D000F0B00000B
to:   02C00041818080800041004180808080001081808080000D000F0B00000B

That corresponds to block = 0x02 <blocktype> where blocktype has changed from being 0x40 (which was correct) to 0xC000 which is incorrect - and is -0x40 in SLEB encoding.

There's something fishy in this code even to begin with - the old code had ExprType::Void = -0x40 when the value to be written out is +0x40, and yet ExprType::I32 = -0x01 was used to write out a value of +0x7F. Somehow the encoding was treating void differently!?!

In the new code, it's been changed to ExprType::Void = +0x40 (corresponding to the byte value that's correct) and ExprType::I32 = +0x7F (again the same as the byte that's to be written out).

We could just revert this commit, or maybe spend another half hour trying to understand how on earth the old code was working...

[ncw]

OK, I get it. The writer is writing out operands as SLEB128 still, not as uint8_t.

So the old code had Void = -0x40, which encodes in SLEB to 0x40, and had I32 = -0x01 which encodes in SLEB to 0x7F.

This patch changed the values to their actual byte values, but they'll be mangled when they go through the SLEB operand writer (WebAssemblyMCCodeEmitter, branch for Info.OperandType == WebAssembly::OPERAND_SIGNATURE).

[ncw]

In D43991#1025115, @aheejin wrote:

This broke test/MC/WebAssembly/global-ctor-dtor.ll in the waterfall. I checked the output but don't have any idea why it's causing differences in relocation and function information (I'm not familiar with the linker). Do you know why?

The relocs were changing because the operand went from 0x40 (correct value) to 0xC000 (SLEB encoding of 0x40). Because the length of the sequence changed, the addresses of all the relocs were shifted.

[ncw]

For now, I'll revert the bad commit; if you want to re-apply it, you'll have to apply this change with it, so that SLEB encoding isn't applied to the uint8_t bytes:

diff --git a/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp b/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp
index 77744e53d62..0d35806229a 100644
--- a/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp
+++ b/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCCodeEmitter.cpp
@@ -93,7 +93,9 @@ void WebAssemblyMCCodeEmitter::encodeInstruction(
         } else if (Info.OperandType == WebAssembly::OPERAND_GLOBAL) {
           llvm_unreachable("wasm globals should only be accessed symbolicly");
         } else if (Info.OperandType == WebAssembly::OPERAND_SIGNATURE) {
-          encodeSLEB128(int64_t(MO.getImm()), OS);
+          assert(MO.getImm() > 0 && (MO.getImm() & ~0x3f) == 0x40 &&
+                 "Signature must be pre-encoded negative single-byte SLEB");
+          OS << uint8_t(MO.getImm());
         } else {
           encodeULEB128(uint64_t(MO.getImm()), OS);
         }

[aheejin]

Thank you so much for looking into this! Resubmitted in D44034.

llvm/trunk/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h

 static const unsigned LoadAddressOperandNo = 3;
 static const unsigned StoreAddressOperandNo = 2;
 
 /// The operand number of the load or store p2align in load/store instructions.
 static const unsigned LoadP2AlignOperandNo = 1;
 static const unsigned StoreP2AlignOperandNo = 0;
 
 /// This is used to indicate block signatures.
-enum class ExprType {
-  Void    = -0x40,
-  I32     = -0x01,
-  I64     = -0x02,
-  F32     = -0x03,
-  F64     = -0x04,
-  I8x16   = -0x05,
-  I16x8   = -0x06,
-  I32x4   = -0x07,
-  F32x4   = -0x08,
-  B8x16   = -0x09,
-  B16x8   = -0x0a,
-  B32x4   = -0x0b
+enum class ExprType : unsigned {
+  Void    = 0x40,
+  I32     = 0x7F,
+  I64     = 0x7E,
+  F32     = 0x7D,
+  F64     = 0x7C,
+  I8x16   = 0x7B,
+  I16x8   = 0x7A,
+  I32x4   = 0x79,
+  F32x4   = 0x78,
+  B8x16   = 0x77,
+  B16x8   = 0x76,
+  B32x4   = 0x75
 };
 
 /// Instruction opcodes emitted via means other than CodeGen.
 static const unsigned Nop = 0x01;
 static const unsigned End = 0x0b;
 
 wasm::ValType toValType(const MVT &Ty);
 
 } // end namespace WebAssembly
 } // end namespace llvm
 
 #endif