[PATCH] D62510: [LoopVectorize] Add FNeg instruction support

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 28 09:48:07 PDT 2019


fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4001
   }
+  case Instruction::FNeg: {
+    // Just widen unary ops.
----------------
The duplication with the binary ops seems a little unfortunate to me. The widening is basically the same, except the number of arguments.

 What would it take to make it generic to support both binary & unary ops? If we would have a builder function CreateNAryOp(Opcode, ArrayRef<Value*>) the generic version would be quite compact I think. The casts to UnaryOperator (and BinaryOperator) should not be necessary, right? For the copyIRFlags case, we only care if VecOp is an instruction?


================
Comment at: llvm/test/Transforms/LoopVectorize/X86/fneg.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt %s -loop-vectorize -force-vector-interleave=1 -S | FileCheck %s
----------------
Can you make the test independent of X86? I think you can just pass `-force-vector-width=4`, as we just want to test the widening, not anything cost-modeling related.


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

https://reviews.llvm.org/D62510





More information about the llvm-commits mailing list