This is an archive of the discontinued LLVM Phabricator instance.

[X86] Store DAGCombine should not assume arbitrary vector types are simple
ClosedPublic

Authored by mkuper on May 11 2015, 6:43 AM.

Details

Summary

This fixes PR23476.
The basic issue is that we have a store being fed by an extract from v16i64. The combine tries to bitcast it into a v16f64, which is not a simple type, even though v16i64 is.
More generally, this may fire before legalization, so the origin type doesn't have to be simple either.

Diff Detail

Repository
rL LLVM

Event Timeline

mkuper updated this revision to Diff 25465.May 11 2015, 6:43 AM
mkuper retitled this revision from to [X86] Store DAGCombine should not assume arbitrary vector types are simple.
mkuper updated this object.
mkuper edited the test plan for this revision. (Show Details)
mkuper added a reviewer: spatel.
mkuper added a subscriber: Unknown Object (MLST).
spatel accepted this revision.May 11 2015, 9:02 AM
spatel edited edge metadata.

Thanks, Michael. The code change LGTM, but can you minimize the test case and add it to the file with the related tests?

I think something like this patch would do:

Index: test/CodeGen/X86/i64-mem-copy.ll
===================================================================
--- test/CodeGen/X86/i64-mem-copy.ll	(revision 236961)
+++ test/CodeGen/X86/i64-mem-copy.ll	(working copy)
@@ -63,3 +63,15 @@
   ret void
 }
 
+; https://llvm.org/bugs/show_bug.cgi?id=23476
+; Handle extraction from a non-simple / pre-legalization type.
+
+define void @PR23476(<5 x i64> %in, i64* %out, i32 %index) {
+; X32-LABEL: PR23476:
+; X32:         movsd {{.*#+}} xmm0 = mem[0],zero
+; X32-NEXT:    movsd %xmm0, (%eax)
+  %ext = extractelement <5 x i64> %in, i32 %index
+  store i64 %ext, i64* %out, align 8
+  ret void
+}
+
This revision is now accepted and ready to land.May 11 2015, 9:02 AM

Sure, I just copied the test-case from the PR. Will use the minimized one.
Thanks!

This revision was automatically updated to reflect the committed changes.