[PATCH] D56520: [WebAssembly] Mask SIMD shift values
Heejin Ahn via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 11 15:16:29 PST 2019
aheejin added inline comments.
================
Comment at: lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1170
+ Op.getOperand(0), MaskedShift)
+ .getNode());
+ }
----------------
Minor thing: It's sometimes hard to remember which is operand 0 and which is 1.. can we have inline comment for some of arguments to `DAG.getNode` and `DAG.UnrollVectorOp`, something like D50277? (It is a not landed yet CL of mine, and I brought this just for an example, but I saw this kind of commenting many other places in LLVM too)
And also we can early-exit i32/i64 case by
```
// 32-bit and 64-bit unrolled shifts will have proper semantics
if (Op.getSimpleValueType().getVectorElementType().bitsGE(MVT::i32))
return DAG.UnrollVectorOp(Op.getNode());
// Mask the operand to get proper semantics from 32-bit shift
SDValue Shift = Op.getOperand(1);
...
```
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56520/new/
https://reviews.llvm.org/D56520
More information about the llvm-commits
mailing list