[PATCH] [X86][FastIsel] Teach how to select vector load instructions.

Quentin Colombet qcolombet at apple.com
Wed Mar 25 12:18:10 PDT 2015


================
Comment at: lib/Target/X86/X86FastISel.cpp:331
@@ -331,1 +330,3 @@
+                                  MachineMemOperand *MMO, unsigned &ResultReg,
+                                  bool Aligned) {
   // Get opcode and regclass of the output for the given load instruction.
----------------
Instead of passing a bool, I think the actual alignment we saw may be useful.
We could imagine that we generate different instructions based on the value of that alignment.

That being said, this is not the case right now, so we can adjust accordingly later. It just feel weird that the callers know about the required alignments.

What do you think?

================
Comment at: lib/Target/X86/X86FastISel.cpp:1099
@@ +1098,3 @@
+  if (Alignment == 0) // Ensure that codegen never sees alignment 0
+    Alignment = ABIAlignment;
+  bool Aligned = Alignment >= ABIAlignment;
----------------
andreadb wrote:
> qcolombet wrote:
> > By default, don’t we assume stuff are unaligned, i.e., Alignment = 1?
> That's what I thought originally. However, when testing this patch, I noticed that without fast-isel I was always getting aligned load instructions.
> 
> Example:
> ```
> define <4 x float> @foo(<4 x float>* %V) {
> entry:
>   %0 = load <4 x float>, <4 x float>* %V
>   ret <4 x float> %0
> }
> ```
> without -fast-isel, llc -mattr=+sse2 generates a vmovaps.
> 
> As far as I understand, if a load doesn't specify the alignment, then the backend assumes the 'ABIAlignment' based on the value type.
> 
> This also matches the behavior of method 'X86FastISel::X86SelectStore'. At around line 934, we check for the alignment of the store in input; if a store node doesn't specify any alignment (i.e. Alignment is 0), then FastISel implicitly sets Alignment to the 'ABIAlignment'.
> For the record: the code between lines 1096 and 1099 is mostly copy-pasted from that method.
> 
> In general I don't have a strong opinion on this: I can change line 1099 so that we propagate value 1 to Alignment. However, if we do this, then for consistency, we should  probably also change how we check the alignment of store instructions in 'X86SelectStore'.  What do you think?
It seems we do the same for SelectionDAG, at least at some places, unlike what I thought.
May be worth bringing that up on the dev mailing list.

Anyhow, this part looks good as it is consistent with what exists.

http://reviews.llvm.org/D8605

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list