[PATCH] D30216: GlobalISel: Translate ConstantDataVector

[Daniel Sanders via Phabricator via llvm-commits]

dsanders accepted this revision.
dsanders added a comment.
This revision is now accepted and ready to land.

This will LGTM for the architectures supported by GlobalISel at the moment with a couple more tests but I think there's targets in LLVM that need different behaviour (PowerPC?). If that's the case, should we account for this now or later?



================
Comment at: lib/CodeGen/GlobalISel/IRTranslator.cpp:988
+      Ops.push_back(getOrCreateVReg(Elt));
+      Indices.push_back(i * (CV->getElementByteSize() * 8));
+    }
----------------
This looks correct for ARM/AArch64/Mips, but I'm not sure this is correct for all targets. I'm thinking of big-endian targets where the highest-numbered element is stored at bit 0 (PowerPC?).


================
Comment at: test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll:1193-1201
+define <2 x i32> @test_constant_datavector() {
+; CHECK-LABEL: name: test_constant_datavector
+; CHECK: [[ONE:%[0-9]+]](s32) = G_CONSTANT i32 1
+; CHECK: [[TWO:%[0-9]+]](s32) = G_CONSTANT i32 2
+; CHECK: [[RES:%[0-9]+]](<2 x s32>) = G_SEQUENCE [[ONE]](s32), 0, [[TWO]](s32), 32
+; CHECK: %d0 = COPY [[RES]](<2 x s32>)
+entry:
----------------
I'd add a couple more tests, one for another vector size (preferably not a power of 2) and one for a different type and size like 'double'.


https://reviews.llvm.org/D30216