[PATCH] D51553: [DAGCombiner][x86] add transform/hook to load a scalar directly for use in a vector binop

[Sanjay Patel via Phabricator via llvm-commits]

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


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:15103-15105
+  const TargetLowering &TLI = DAG.getTargetLoweringInfo();
+  if (!TLI.shouldLoadScalarIntoVectorOp(SDValue(Ins, 0), BO))
+    return SDValue();
----------------
andreadb wrote:
> I wonder if this should be disabled for optsize/minsize.
> On x86, this is going to increase .text, and it also potentially introduces a new constant in the pool.
> 
> ```
> mov (%rdi), %eax
> add $0x2a, %eax
> vmovd %eax, %xmm0
> retq
> ```
> 
> is 10 bytes.
> 
> ```
> vmovd (%rdi), %xmm0
> vpaddd 0x0(%rip), %xmm0, %xmm0  ## 4 bytes PC relative relocation
> retq
> ```
> 
> is 13 bytes.
> 
> Plus, 16B in the constant pool (if we didn't see that constant before)...
Yes, that seems like a good constraint for x86. I'm not sure that other targets would want the same limitation though, so I think we should make that another option in the target hook itself rather than here in generic code.


https://reviews.llvm.org/D51553