[PATCH] D53625: [WebAssembly] General vector shift lowering

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added inline comments.


================
Comment at: lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1027
+  if (!Op.getSimpleValueType().isVector())
     return Op;
+
----------------
We did `setOperationAction` to `Custom` only for vector types, so is there a case this `if` can be satisfied? How about changing this to an assertion this is a vector type?


================
Comment at: lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1034
+      !(SplatVal = ShiftVec->getSplatValue()))
+    return DAG.UnrollVectorOp(Op.getNode());
+
----------------
On an unrelated note, maybe we can use this function again to conditionally expand `extract_lane`?


================
Comment at: lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1036
+
+  // Non-const splats are handled by patterns, except for i64x2
+  ConstantSDNode *SplatConst = dyn_cast<ConstantSDNode>(SplatVal);
----------------
Do we have test cases for non-const splat?


================
Comment at: lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1055
     llvm_unreachable("unexpected opcode");
     return Op;
   }
----------------
This isn't added in this CL, but we probably don't need this line after `llvm_unreachable`


Repository:
  rL LLVM

https://reviews.llvm.org/D53625