[PATCH] D61823: [WebAssembly] Don't assume that zext/sext result is i32/i64 in fast isel (PR41841)

Thomas Lively via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun May 12 16:56:55 PDT 2019


tlively accepted this revision.
tlively added a comment.
This revision is now accepted and ready to land.

Thanks for fixing this! I have a few comments on the test style, but otherwise LGTM.



================
Comment at: llvm/test/CodeGen/WebAssembly/PR41841.ll:1
+; RUN: llc < %s -O0 -wasm-disable-explicit-locals -wasm-keep-registers | FileCheck %s
+
----------------
We usually pass `-asm-verbose=false` in our tests to avoid having to match uninteresting output.


================
Comment at: llvm/test/CodeGen/WebAssembly/PR41841.ll:18
+; CHECK-NEXT:	call    	foo, $[[TMP1]], $[[TMP2]]{{$}}
+; CHECK-NEXT:	return{{$}}
+;
----------------
Please move the CHECK lines to be right before the top of the function


================
Comment at: llvm/test/CodeGen/WebAssembly/PR41841.ll:36
+; CHECK-NEXT:	i64.const	$[[TMP6:[0-9]+]]=, 0{{$}}
+; CHECK-NEXT: i64.sub 	$[[TMP1:[0-9]+]]=, $[[TMP6]], $[[TMP5]]{{$}}
+; CHECK-NEXT: local.copy	$[[TMP2:[0-9]+]]=, $[[TMP1]]{{$}}
----------------
Please standardize the whitespace to make the start of the instructions line up.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61823/new/

https://reviews.llvm.org/D61823





More information about the llvm-commits mailing list