[PATCH] D60852: Fix for bug 41512: lower INSERT_VECTOR_ELT(ZeroVec, 0, Elt) to SCALAR_TO_VECTOR(Elt) for all SSE flavors

Serge Preis via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 29 23:59:50 PDT 2019


Serge_Preis marked an inline comment as done.
Serge_Preis added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:16980
+    if (EltVT == MVT::i32 || EltVT == MVT::f32 || EltVT == MVT::f64 ||
+        (EltVT == MVT::i64 && Subtarget.is64Bit())) {
+      N1 = DAG.getNode(ISD::SCALAR_TO_VECTOR, dl, VT, N1);
----------------
craig.topper wrote:
> Serge_Preis wrote:
> > RKSimon wrote:
> > > Subtarget.is64Bit() should be superfluous
> > This is a good question, On one (conservative) hand I am aiming at movq instruction, whch is only available on 64bit subtarget (and similar places have same set of checks). On other hand the logic behind the conversion is pretty generic and doesn't depend on subtarget.. So the question is "are we sure that 64bit SCALAR_TO_VECTOR will be handled efficiently now on 32bit subtarget or such generalization will require further patching?"
> Type legalization would have guaranteed there are no i64 values in the DAG if this is not a 64-bit target before we get to this code.
Ok. uploaded new iteration without subtarget check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60852





More information about the llvm-commits mailing list