[WebAssembly] memory.fill

[tlively]

Summary

memset lowering, fix argument types in memcpy lowering, and
test encodings. Depends on D57736.

[sbc100]

Nice! I like the use of .s file for testing too!

llvm/test/MC/WebAssembly/bulk-memory-encodings.s

18 ↗

(On Diff #185416)

I think this should just be end. I think end_* variants are supposed to be internal. Might want to check with wouter though.

[aheejin]

Looks OK to me, some nits and questions:

llvm/lib/Target/WebAssembly/WebAssemblyInstrBulkMemory.td

36 ↗

(On Diff #185416)

Nit: whitespace between = and 1

48 ↗

(On Diff #185416)

Why is it mayStore? I guess even it does not modify wasm linear memory it can possibly modify other parts of the memory that stores segments, is that right?

llvm/lib/Target/WebAssembly/WebAssemblySelectionDAGInfo.cpp

57 ↗

(On Diff #185416)

Why use getAnyExtOrTrunc for Val and getZExtOrTrunc for Size? What's the difference?

llvm/test/CodeGen/WebAssembly/bulk-memory-intrinsics.ll

15 ↗

(On Diff #185416)

Nit: how about merging it in D57736?

llvm/test/CodeGen/WebAssembly/bulk-memory.ll

46 ↗

(On Diff #185416)

In this file, some val and len arguments have zeroext attribute and some don't. Is there a reason? And do why we need this attribute?

[tlively]

llvm/lib/Target/WebAssembly/WebAssemblyInstrBulkMemory.td

48 ↗

(On Diff #185416)

I wouldn't consider touching the system memory that stores segments to be mayStore, since the WebAssembly semantics cannot reason about that memory. The reason I marked this mayStore is to prevent it from being moved past memory.init instructions, which are able to observe the effects of data.drop by trapping on dropped segments. This is kind of using a large hammer to solve a small problem, but the only alternative I know of is 'hasSideEffects` which seems like an even bigger hammer.

llvm/lib/Target/WebAssembly/WebAssemblySelectionDAGInfo.cpp

57 ↗

(On Diff #185416)

The semantics of memory.fill say that the value argument is taken to be the byte value that memory is filled with, so there is an implicit truncate in the operation itself. That means that it doesn't matter how the value is extended if it starts off at least as large as an i8. All the bits of the size argument are meaningful, though, so it would be bad if it were extended with anything except zeroes.

I suppose this is broken if Val is an i1, so I will fix that.

llvm/test/CodeGen/WebAssembly/bulk-memory.ll

46 ↗

(On Diff #185416)

Only i8 arguments have the zeroext attribute. Without it, the function does extra work to truncate and sign extend the argument, but I don't want that extra complexity in the test case.

[aheejin]

llvm/lib/Target/WebAssembly/WebAssemblyInstrBulkMemory.td

48 ↗

(On Diff #185416)

I think hasSideEffects is OK here because 1. It actually has side effects and 2. Given that data.drop is not gonna be in the middle of some user function, that wouldn't hinder optimization too much anyway.

llvm/lib/Target/WebAssembly/WebAssemblySelectionDAGInfo.cpp

57 ↗

(On Diff #185416)

I see. A bit of comments on that would also be better I think.

llvm/test/CodeGen/WebAssembly/bulk-memory.ll

46 ↗

(On Diff #185416)

I see. But there seem to be other i8 val or len arguments without zeroext attribute in this file.

[tlively]

• Address comments

llvm/lib/Target/WebAssembly/WebAssemblySelectionDAGInfo.cpp

57 ↗

(On Diff #185416)

I forgot that llvm.memset always takes an i8 so this was already correct. I added a comment though.

llvm/test/CodeGen/WebAssembly/bulk-memory.ll

46 ↗

(On Diff #185416)

Yes, it doesn't make a difference for i32 arguments so I just left it out.

llvm/test/MC/WebAssembly/bulk-memory-encodings.s

18 ↗

(On Diff #185416)

I tried this out but the assembler didn't like it.

llvm/trunk/lib/Target/WebAssembly/WebAssemblyISD.def

 HANDLE_NODETYPE(BR_IF)
 HANDLE_NODETYPE(BR_TABLE)
 HANDLE_NODETYPE(SHUFFLE)
 HANDLE_NODETYPE(VEC_SHL)
 HANDLE_NODETYPE(VEC_SHR_S)
 HANDLE_NODETYPE(VEC_SHR_U)
 HANDLE_NODETYPE(THROW)
 HANDLE_NODETYPE(MEMORY_COPY)
+HANDLE_NODETYPE(MEMORY_FILL)
 
 // add memory opcodes starting at ISD::FIRST_TARGET_MEMORY_OPCODE here...

llvm/trunk/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp

WebAssemblyTargetLowering::WebAssemblyTargetLowering(
 
   // Exception handling intrinsics
   setOperationAction(ISD::INTRINSIC_WO_CHAIN, MVT::Other, Custom);
   setOperationAction(ISD::INTRINSIC_VOID, MVT::Other, Custom);
 
   setMaxAtomicSizeInBitsSupported(64);
 
   if (Subtarget->hasBulkMemory()) {
-    // Using memory.copy is always better than using multiple loads and stores
+    // Use memory.copy and friends over multiple loads and stores
     MaxStoresPerMemcpy = 1;
     MaxStoresPerMemcpyOptSize = 1;
     MaxStoresPerMemmove = 1;
     MaxStoresPerMemmoveOptSize = 1;
+    MaxStoresPerMemset = 1;
+    MaxStoresPerMemsetOptSize = 1;
   }
 }
 
 TargetLowering::AtomicExpansionKind
 WebAssemblyTargetLowering::shouldExpandAtomicRMWInIR(AtomicRMWInst *AI) const {
   // We have wasm instructions for these
   switch (AI->getOperation()) {
   case AtomicRMWInst::Add:

llvm/trunk/lib/Target/WebAssembly/WebAssemblyInstrBulkMemory.td

 
 // Bespoke types and nodes for bulk memory ops
 def wasm_memcpy_t : SDTypeProfile<0, 5,
   [SDTCisInt<0>, SDTCisInt<1>, SDTCisPtrTy<2>, SDTCisPtrTy<3>, SDTCisInt<4>]
 >;
 def wasm_memcpy : SDNode<"WebAssemblyISD::MEMORY_COPY", wasm_memcpy_t,
                          [SDNPHasChain, SDNPMayLoad, SDNPMayStore]>;
 
-//===----------------------------------------------------------------------===//
-// memory.init
-//===----------------------------------------------------------------------===//
+def wasm_memset_t : SDTypeProfile<0, 4,
+  [SDTCisInt<0>, SDTCisPtrTy<1>, SDTCisInt<2>, SDTCisInt<3>]
+>;
+def wasm_memset : SDNode<"WebAssemblyISD::MEMORY_FILL", wasm_memset_t,
+                         [SDNPHasChain, SDNPMayStore]>;
 
-let mayStore = 1 in
+let mayStore = 1, hasSideEffects = 1 in
 defm MEMORY_INIT :
   BULK_I<(outs),
          (ins i32imm_op:$seg, i32imm_op:$idx, I32:$dest,
               I32:$offset, I32:$size),
          (outs), (ins i32imm_op:$seg, i32imm_op:$idx),
          [(int_wasm_memory_init (i32 imm:$seg), (i32 imm:$idx), I32:$dest,
             I32:$offset, I32:$size
           )],
          "memory.init\t$seg, $idx, $dest, $offset, $size",
          "memory.init\t$seg, $idx", 0x08>;
 
-//===----------------------------------------------------------------------===//
-// data.drop
-//===----------------------------------------------------------------------===//
-
+let hasSideEffects = 1 in
 defm DATA_DROP :
   BULK_I<(outs), (ins i32imm_op:$seg), (outs), (ins i32imm_op:$seg),
          [(int_wasm_data_drop (i32 imm:$seg))],
          "data.drop\t$seg", "data.drop\t$seg", 0x09>;
 
-//===----------------------------------------------------------------------===//
-// memory.copy
-//===----------------------------------------------------------------------===//
-
 let mayLoad = 1, mayStore = 1 in
 defm MEMORY_COPY :
   BULK_I<(outs), (ins i32imm_op:$src_idx, i32imm_op:$dst_idx,
                       I32:$dst, I32:$src, I32:$len),
          (outs), (ins i32imm_op:$src_idx, i32imm_op:$dst_idx),
          [(wasm_memcpy (i32 imm:$src_idx), (i32 imm:$dst_idx),
            I32:$dst, I32:$src, I32:$len
          )],
          "memory.copy\t$src_idx, $dst_idx, $dst, $src, $len",
          "memory.copy\t$src_idx, $dst_idx", 0x0a>;
+
+let mayStore = 1 in
+defm MEMORY_FILL :
+  BULK_I<(outs), (ins i32imm_op:$idx, I32:$dst, I32:$value, I32:$size),
+         (outs), (ins i32imm_op:$idx),
+         [(wasm_memset (i32 imm:$idx), I32:$dst, I32:$value, I32:$size)],
+         "memory.fill\t$idx, $dst, $value, $size",
+         "memory.fill\t$idx", 0x0b>;

llvm/trunk/lib/Target/WebAssembly/WebAssemblySelectionDAGInfo.h

SDValue EmitTargetCodeForMemcpy(SelectionDAG &DAG, const SDLoc &dl,
                                   bool AlwaysInline,
                                   MachinePointerInfo DstPtrInfo,
                                   MachinePointerInfo SrcPtrInfo) const override;
   SDValue EmitTargetCodeForMemmove(SelectionDAG &DAG, const SDLoc &dl,
                                    SDValue Chain, SDValue Op1, SDValue Op2,
                                    SDValue Op3, unsigned Align, bool isVolatile,
                                    MachinePointerInfo DstPtrInfo,
                                    MachinePointerInfo SrcPtrInfo) const override;
+  SDValue EmitTargetCodeForMemset(SelectionDAG &DAG, const SDLoc &DL,
+                                  SDValue Chain, SDValue Op1, SDValue Op2,
+                                  SDValue Op3, unsigned Align, bool IsVolatile,
+                                  MachinePointerInfo DstPtrInfo) const override;
 };
 
 } // end namespace llvm
 
 #endif

llvm/trunk/lib/Target/WebAssembly/WebAssemblySelectionDAGInfo.cpp

 SDValue WebAssemblySelectionDAGInfo::EmitTargetCodeForMemmove(
     SelectionDAG &DAG, const SDLoc &DL, SDValue Chain, SDValue Op1, SDValue Op2,
     SDValue Op3, unsigned Align, bool IsVolatile,
     MachinePointerInfo DstPtrInfo, MachinePointerInfo SrcPtrInfo) const {
   return EmitTargetCodeForMemcpy(DAG, DL, Chain, Op1, Op2, Op3, Align,
                                  IsVolatile, false, DstPtrInfo,
                                  SrcPtrInfo);
 }
+
+SDValue WebAssemblySelectionDAGInfo::EmitTargetCodeForMemset(
+    SelectionDAG &DAG, const SDLoc &DL, SDValue Chain, SDValue Dst, SDValue Val,
+    SDValue Size, unsigned Align, bool IsVolatile,
+    MachinePointerInfo DstPtrInfo) const {
+  if (!DAG.getMachineFunction()
+           .getSubtarget<WebAssemblySubtarget>()
+           .hasBulkMemory())
+    return SDValue();
+
+  SDValue MemIdx = DAG.getConstant(0, DL, MVT::i32);
+  // Only low byte matters for val argument, so anyext the i8
+  return DAG.getNode(WebAssemblyISD::MEMORY_FILL, DL, MVT::Other, Chain, MemIdx,
+                     Dst, DAG.getAnyExtOrTrunc(Val, DL, MVT::i32),
+                     DAG.getZExtOrTrunc(Size, DL, MVT::i32));
+}

llvm/trunk/test/CodeGen/WebAssembly/bulk-memory.ll

 ; RUN: llc < %s -asm-verbose=false -verify-machineinstrs -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -mattr=+bulk-memory | FileCheck %s --check-prefixes CHECK,BULK-MEM
 ; RUN: llc < %s -asm-verbose=false -verify-machineinstrs -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -mattr=-bulk-memory | FileCheck %s --check-prefixes CHECK,NO-BULK-MEM
 
 ; Test that basic bulk memory codegen works correctly
 
 target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
 target triple = "wasm32-unknown-unknown"
 
 declare void @llvm.memcpy.p0i8.p0i8.i8(i8*, i8*, i8, i1)
 declare void @llvm.memcpy.p0i8.p0i8.i32(i8*, i8*, i32, i1)
 declare void @llvm.memcpy.p0i32.p0i32.i32(i32*, i32*, i32, i1)
 
 declare void @llvm.memmove.p0i8.p0i8.i8(i8*, i8*, i8, i1)
 declare void @llvm.memmove.p0i8.p0i8.i32(i8*, i8*, i32, i1)
 declare void @llvm.memmove.p0i32.p0i32.i32(i32*, i32*, i32, i1)
 
+declare void @llvm.memset.p0i8.i8(i8*, i8, i8, i1)
+declare void @llvm.memset.p0i8.i32(i8*, i8, i32, i1)
+declare void @llvm.memset.p0i32.i32(i32*, i8, i32, i1)
+
 ; CHECK-LABEL: memcpy_i8:
 ; NO-BULK-MEM-NOT: memory.copy
 ; BULK-MEM-NEXT: .functype memcpy_i8 (i32, i32, i32) -> ()
 ; BULK-MEM-NEXT: memory.copy 0, 0, $0, $1, $2
 ; BULK-MEM-NEXT: return
 define void @memcpy_i8(i8* %dest, i8* %src, i8 zeroext %len) {
   call void @llvm.memcpy.p0i8.p0i8.i8(i8* %dest, i8* %src, i8 %len, i1 0)
   ret void
 }
 
 ; CHECK-LABEL: memmove_i8:
 ; NO-BULK-MEM-NOT: memory.copy
 ; BULK-MEM-NEXT: .functype memmove_i8 (i32, i32, i32) -> ()
 ; BULK-MEM-NEXT: memory.copy 0, 0, $0, $1, $2
 ; BULK-MEM-NEXT: return
 define void @memmove_i8(i8* %dest, i8* %src, i8 zeroext %len) {
   call void @llvm.memmove.p0i8.p0i8.i8(i8* %dest, i8* %src, i8 %len, i1 0)
   ret void
 }
 
+; CHECK-LABEL: memset_i8:
+; NO-BULK-MEM-NOT: memory.fill
+; BULK-MEM-NEXT: .functype memset_i8 (i32, i32, i32) -> ()
+; BULK-MEM-NEXT: memory.fill 0, $0, $1, $2
+; BULK-MEM-NEXT: return
+define void @memset_i8(i8* %dest, i8 %val, i8 zeroext %len) {
+  call void @llvm.memset.p0i8.i8(i8* %dest, i8 %val, i8 %len, i1 0)
+  ret void
+}
+
 ; CHECK-LABEL: memcpy_i32:
 ; NO-BULK-MEM-NOT: memory.copy
 ; BULK-MEM-NEXT: .functype memcpy_i32 (i32, i32, i32) -> ()
 ; BULK-MEM-NEXT: memory.copy 0, 0, $0, $1, $2
 ; BULK-MEM-NEXT: return
 define void @memcpy_i32(i32* %dest, i32* %src, i32 %len) {
   call void @llvm.memcpy.p0i32.p0i32.i32(i32* %dest, i32* %src, i32 %len, i1 0)
   ret void
 }
 
 ; CHECK-LABEL: memmove_i32:
 ; NO-BULK-MEM-NOT: memory.copy
 ; BULK-MEM-NEXT: .functype memmove_i32 (i32, i32, i32) -> ()
 ; BULK-MEM-NEXT: memory.copy 0, 0, $0, $1, $2
 ; BULK-MEM-NEXT: return
 define void @memmove_i32(i32* %dest, i32* %src, i32 %len) {
   call void @llvm.memmove.p0i32.p0i32.i32(i32* %dest, i32* %src, i32 %len, i1 0)
   ret void
 }
 
+; CHECK-LABEL: memset_i32:
+; NO-BULK-MEM-NOT: memory.fill
+; BULK-MEM-NEXT: .functype memset_i32 (i32, i32, i32) -> ()
+; BULK-MEM-NEXT: memory.fill 0, $0, $1, $2
+; BULK-MEM-NEXT: return
+define void @memset_i32(i32* %dest, i8 %val, i32 %len) {
+  call void @llvm.memset.p0i32.i32(i32* %dest, i8 %val, i32 %len, i1 0)
+  ret void
+}
+
 ; CHECK-LABEL: memcpy_1:
 ; CHECK-NEXT: .functype memcpy_1 (i32, i32) -> ()
 ; CHECK-NEXT: i32.load8_u $push[[L0:[0-9]+]]=, 0($1)
 ; CHECK-NEXT: i32.store8 0($0), $pop[[L0]]
 ; CHECK-NEXT: return
 define void @memcpy_1(i8* %dest, i8* %src) {
   call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dest, i8* %src, i32 1, i1 0)
   ret void
 }
 
 ; CHECK-LABEL: memmove_1:
 ; CHECK-NEXT: .functype memmove_1 (i32, i32) -> ()
 ; CHECK-NEXT: i32.load8_u $push[[L0:[0-9]+]]=, 0($1)
 ; CHECK-NEXT: i32.store8 0($0), $pop[[L0]]
 ; CHECK-NEXT: return
 define void @memmove_1(i8* %dest, i8* %src) {
   call void @llvm.memmove.p0i8.p0i8.i32(i8* %dest, i8* %src, i32 1, i1 0)
   ret void
 }
 
+; CHECK-LABEL: memset_1:
+; NO-BULK-MEM-NOT: memory.fill
+; BULK-MEM-NEXT: .functype memset_1 (i32, i32) -> ()
+; BULK-MEM-NEXT: i32.store8 0($0), $1
+; BULK-MEM-NEXT: return
+define void @memset_1(i8* %dest, i8 %val) {
+  call void @llvm.memset.p0i8.i32(i8* %dest, i8 %val, i32 1, i1 0)
+  ret void
+}
+
 ; CHECK-LABEL: memcpy_1024:
 ; NO-BULK-MEM-NOT: memory.copy
 ; BULK-MEM-NEXT: .functype memcpy_1024 (i32, i32) -> ()
 ; BULK-MEM-NEXT: i32.const $push[[L0:[0-9]+]]=, 1024
 ; BULK-MEM-NEXT: memory.copy 0, 0, $0, $1, $pop[[L0]]
 ; BULK-MEM-NEXT: return
 define void @memcpy_1024(i8* %dest, i8* %src) {
   call void @llvm.memcpy.p0i8.p0i8.i32(i8* %dest, i8* %src, i32 1024, i1 0)
   ret void
 }
 
 ; CHECK-LABEL: memmove_1024:
 ; NO-BULK-MEM-NOT: memory.copy
 ; BULK-MEM-NEXT: .functype memmove_1024 (i32, i32) -> ()
 ; BULK-MEM-NEXT: i32.const $push[[L0:[0-9]+]]=, 1024
 ; BULK-MEM-NEXT: memory.copy 0, 0, $0, $1, $pop[[L0]]
 ; BULK-MEM-NEXT: return
 define void @memmove_1024(i8* %dest, i8* %src) {
   call void @llvm.memmove.p0i8.p0i8.i32(i8* %dest, i8* %src, i32 1024, i1 0)
   ret void
 }
+
+; CHECK-LABEL: memset_1024:
+; NO-BULK-MEM-NOT: memory.fill
+; BULK-MEM-NEXT: .functype memset_1024 (i32, i32) -> ()
+; BULK-MEM-NEXT: i32.const $push[[L0:[0-9]+]]=, 1024
+; BULK-MEM-NEXT: memory.fill 0, $0, $1, $pop[[L0]]
+; BULK-MEM-NEXT: return
+define void @memset_1024(i8* %dest, i8 %val) {
+  call void @llvm.memset.p0i8.i32(i8* %dest, i8 %val, i32 1024, i1 0)
+  ret void
+}

llvm/trunk/test/MC/WebAssembly/bulk-memory-encodings.s

+# RUN: llvm-mc -show-encoding -triple=wasm32-unkown-unknown -mattr=+bulk-memory < %s | FileCheck %s
+
+main:
+    .functype main () -> ()
+
+    # CHECK: memory.init 3, 0 # encoding: [0xfc,0x08,0x03,0x00]
+    memory.init 3, 0
+
+    # CHECK: data.drop 3 # encoding: [0xfc,0x09,0x03]
+    data.drop 3
+
+    # CHECK: memory.copy 0, 0 # encoding: [0xfc,0x0a,0x00,0x00]
+    memory.copy 0, 0
+
+    # CHECK: memory.fill 0 # encoding: [0xfc,0x0b,0x00]
+    memory.fill 0
+
+    end_function