[PATCH] [x86] Combine x86mmx/i64 to v2i64 conversion to use scalar_to_vector

Bruno Cardoso Lopes bruno.cardoso at gmail.com
Fri Jan 23 15:25:40 PST 2015


Hi,

On Fri, Jan 23, 2015 at 9:13 PM, Chandler Carruth <chandlerc at gmail.com> wrote:
> Generally nice. Some further comments to spatel's inline. I'm happy with the change once spatel is, and with whatever approach you take for cleaning up the test cases.
>
>
> REPOSITORY
>   rL LLVM
>
> ================
> Comment at: lib/Target/X86/X86ISelLowering.cpp:24754
> @@ +24753,3 @@
> +
> +    // On 32bit systems, we can't save 64bit integers, use f64 instead
> +    bool Usef64 = TLI.isTypeLegal(MVT::f64) && !Subtarget->is64Bit();
> ----------------
> spatel wrote:
>> Period at end of sentence.
> Also, I would use "store" rather than "save".

Ok

> ================
> Comment at: test/CodeGen/X86/2012-01-18-vbitcast.ll:5
> @@ -4,3 +4,3 @@
>  define <2 x i32> @vcast(<2 x float> %a, <2 x float> %b) {
> -;CHECK: pmovzxdq
> -;CHECK: pmovzxdq
> +;CHECK-NOT: pmovzxdq
> +;CHECK-NOT: pmovzxdq
> ----------------
> spatel wrote:
>> I was told that we should always put a space between the ';' and 'CHECK'. And yes, I realize that a very large fraction of the existing regression tests do not do this. :)
> My preference is to convert tests to use the utils/update_llc_test_checks.py script when appropriate (IE, when they are testing a microscopic and very specific sequence of instructions). Where we can, I'd also appreciate merging test cases out of <date>-foo.ll files and into reasonably semantically named files.
>
> I'm not too fussed about what order this stuff happens in, but I usually find it easier to review patches when the test cases are moved or regularized with the script first as it tends to make the diff cleaner.
>
> Anyways, not super important to this *specific* patch, but hopefully useful going forward.

Should have mentioned that I'll likely be playing around to optimize
some other mmx constructs, rewriting the tests in on my plans. It's a
nice thing to do indeed.

> ================
> Comment at: test/CodeGen/X86/mmx-movq2dq.ll:1
> @@ +1,2 @@
> +; RUN: llc < %s -march=x86 -mattr=+mmx,+sse2 | FileCheck %s -check-prefix=X86-32
> +; RUN: llc < %s -march=x86-64 -mattr=+mmx,+sse2 | FileCheck %s -check-prefix=X86-64
> ----------------
> How about a generic 'vector-shuffle-mmx.ll' test file that we can consolidate MMX-specific shuffle and mov test cases into?

Perfect. I went ahead and committed, will do you suggestions in
upcoming commits then.

Thanks again Chandler,

-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc




More information about the llvm-commits mailing list