[PATCH] D54698: [SelectionDAG] Initial support for FSHL/FSHR funnel shift opcodes (PR39467)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a reviewer: fabiang.
spatel added a comment.

I just see a few inline nits. Adding Fabian in case he has any feedback.



================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:1168
+
+  // Otherwise go ahead and unroll.
+  return DAG.UnrollVectorOp(Op.getNode());
----------------
I realize this comment is copied, but it wasn't really adding value the first 3 times we said it either. :)


================
Comment at: test/CodeGen/X86/funnel-shift.ll:17
 
 ; General case - all operands can be variables - x86 has shld, but the mask and cmov are not needed?
 
----------------
Update test comment - no mask/cmov needed.


================
Comment at: test/CodeGen/X86/funnel-shift.ll:207
 
 ; General case - all operands can be variables - x86 has 'shrd', but the mask and cmov are not needed?
 
----------------
Update test comment - no mask/cmov needed.


================
Comment at: test/CodeGen/X86/funnel-shift.ll:340
 
 ; Check modulo math on shift amount. 41-32=9, but right-shift became left, so 32-9=23.
 
----------------
Update test comment? Do you know why we got opposite directions here?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54698