GlobalISel: Translate ConstantDataVector

[dsanders]

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?

lib/CodeGen/GlobalISel/IRTranslator.cpp

1074

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?).

test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll

1248–1256

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'.

[kristof.beyls]

lib/CodeGen/GlobalISel/IRTranslator.cpp

1074

If this is not correct for some targets, there should at list be a FIXME here explaining in what circumstances the code isn't correct?

test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll

1248–1256

My guess is that non-power-of-2 vector sizes are too ill-supported to be able to write tests using them at the moment?
I'm currently starting to look into supporting non-power-of-2-sized types - it's looking like it may be quite a bit of work.

[ab]

I'm not sure we want to use G_SEQUENCE for this. It seems to me that we should figure out a representation for vectors in general before diving into constants.

lib/CodeGen/GlobalISel/IRTranslator.cpp

1074

This isn't always a valid assumption; we can have things like v16i1.

Make this size in bits instead?

[dsanders]

lib/CodeGen/GlobalISel/IRTranslator.cpp

1074

This isn't always a valid assumption; we can have things like v16i1.

v16i1 can't occur here. ConstantDataVector handles the common case vectors where the elements are i8/i16/i32/i64/half/float/double. The v16i1 case is handled by ConstantVector instead.

[qcolombet]

lib/CodeGen/GlobalISel/IRTranslator.cpp

1074

A genuine question, does SD care about the endian distinction for building vectors?
The reason I am asking is because I could see this being the target dealing with that at selection time, so I want to make sure we are consistent with whatever SD was doing.

1076

For this case we should use G_MERGE.

test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll

1248–1256

For the IRTranslator non-power-of-2 vectors should be fine.
At least I don't see any difficulty for this specific pass.

[dsanders]

lib/CodeGen/GlobalISel/IRTranslator.cpp

1074

I don't think it matters to the SelectionDAG representation but I believe it's important to the transition between SelectionDAG nodes to MachineInstrs. Mips was handling the bitconvert differences at instruction selection time. I'm not familiar with the PowerPC instruction set but it looks like PowerPC is doing some endian aware lowering of BUILD_VECTOR in PPCTargetLowering::LowerBUILD_VECTOR().

I suppose the key question to answer is: Do gMIR's virtual registers hold SelectionDAG values or physical register values?

[volkan]

lib/CodeGen/GlobalISel/IRTranslator.cpp

1074

SelectionDAG uses the same order to build vector.

[volkan]

Use G_MERGE_VALUES to represent vectors.

[qcolombet]

LGTM + test case

test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll

1248–1256

Could you add a non-power-of-2 test case?

[volkan]

Added a non-power-of-2 test.

[ab]

This approach makes sense to me too, thanks folks! If no one else has a paragraph handy, I'll try to explain the whole post-G_SEQUENCE scheme in the docs page.

test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll

1341–1349

Any reason for this test to involve an extractelement?

[volkan]

test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll

1341–1349

We are unable to translate ret <3 x i32> <...>, so I tried something else.

lib/CodeGen/GlobalISel/IRTranslator.cpp

bool IRTranslator::translate(const Constant &C, unsigned Reg) {
   else if (auto GV = dyn_cast<GlobalValue>(&C))
     EntryBuilder.buildGlobalValue(Reg, GV);
   else if (auto CAZ = dyn_cast<ConstantAggregateZero>(&C)) {
     if (!CAZ->getType()->isVectorTy())
       return false;
     std::vector<unsigned> Ops;
     for (unsigned i = 0; i < CAZ->getNumElements(); ++i) {
       Constant &Elt = *CAZ->getElementValue(i);
       Ops.push_back(getOrCreateVReg(Elt));
     }
     EntryBuilder.buildMerge(Reg, Ops);
+  } else if (auto CV = dyn_cast<ConstantDataVector>(&C)) {
+    std::vector<unsigned> Ops;
+    for (unsigned i = 0; i < CV->getNumElements(); ++i) {
+      Constant &Elt = *CV->getElementAsConstant(i);
+      Ops.push_back(getOrCreateVReg(Elt));
+    }
+    EntryBuilder.buildMerge(Reg, Ops);
   } else if (auto CE = dyn_cast<ConstantExpr>(&C)) {
     switch(CE->getOpcode()) {
 #define HANDLE_INST(NUM, OPCODE, CLASS)                         \
       case Instruction::OPCODE: return translate##OPCODE(*CE, EntryBuilder);
 #include "llvm/IR/Instruction.def"
     default:
       return false;
     }

test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll

; CHECK: G_STORE [[V2]](s8), [[ADDR]](p0) :: (store singlethread monotonic 1 into %ir.addr)
   store atomic i8 %v1, i8* %addr release, align 1
 
   %v2 = load atomic i8, i8* %addr singlethread seq_cst, align 1
   store atomic i8 %v2, i8* %addr singlethread monotonic, align 1
 
   ret void
 }
 
 define float @test_fneg_f32(float %x) {
 ; CHECK-LABEL: name: test_fneg_f32
 ; CHECK: [[ARG:%[0-9]+]](s32) = COPY %s0
 ; CHECK: [[RES:%[0-9]+]](s32) = G_FNEG [[ARG]]
 ; CHECK: %s0 = COPY [[RES]](s32)
   %neg = fsub float -0.000000e+00, %x
   ret float %neg
 }
 
 define double @test_fneg_f64(double %x) {
 ; CHECK-LABEL: name: test_fneg_f64
 ; CHECK: [[ARG:%[0-9]+]](s64) = COPY %d0
 ; CHECK: [[RES:%[0-9]+]](s64) = G_FNEG [[ARG]]
 ; CHECK: %d0 = COPY [[RES]](s64)
   %neg = fsub double -0.000000e+00, %x
   ret double %neg
 }

 define i32 @test_constantaggzerovector_v3i32() {
 ; CHECK-LABEL: name: test_constantaggzerovector_v3i32
 ; CHECK: [[ZERO:%[0-9]+]](s32) = G_CONSTANT i32 0
 ; CHECK: [[VEC:%[0-9]+]](<3 x s32>) = G_MERGE_VALUES [[ZERO]](s32), [[ZERO]](s32), [[ZERO]](s32)
 ; CHECK: G_EXTRACT_VECTOR_ELT [[VEC]](<3 x s32>)
   %elt = extractelement <3 x i32> zeroinitializer, i32 1
   ret i32 %elt
 }
+
+define <2 x i32> @test_constantdatavector_v2i32() {
+; CHECK-LABEL: name: test_constantdatavector_v2i32
+; CHECK: [[C1:%[0-9]+]](s32) = G_CONSTANT i32 1
+; CHECK: [[C2:%[0-9]+]](s32) = G_CONSTANT i32 2
+; CHECK: [[VEC:%[0-9]+]](<2 x s32>) = G_MERGE_VALUES [[C1]](s32), [[C2]](s32)
+; CHECK: %d0 = COPY [[VEC]](<2 x s32>)
+  ret <2 x i32> <i32 1, i32 2>
+}
+
+define i32 @test_constantdatavector_v3i32() {
+; CHECK-LABEL: name: test_constantdatavector_v3i32
+; CHECK: [[C1:%[0-9]+]](s32) = G_CONSTANT i32 1
+; CHECK: [[C2:%[0-9]+]](s32) = G_CONSTANT i32 2
+; CHECK: [[C3:%[0-9]+]](s32) = G_CONSTANT i32 3
+; CHECK: [[VEC:%[0-9]+]](<3 x s32>) = G_MERGE_VALUES [[C1]](s32), [[C2]](s32), [[C3]](s32)
+; CHECK: G_EXTRACT_VECTOR_ELT [[VEC]](<3 x s32>)
+  %elt = extractelement <3 x i32> <i32 1, i32 2, i32 3>, i32 1
+  ret i32 %elt
+}
+
+define <4 x i32> @test_constantdatavector_v4i32() {
+; CHECK-LABEL: name: test_constantdatavector_v4i32
+; CHECK: [[C1:%[0-9]+]](s32) = G_CONSTANT i32 1
+; CHECK: [[C2:%[0-9]+]](s32) = G_CONSTANT i32 2
+; CHECK: [[C3:%[0-9]+]](s32) = G_CONSTANT i32 3
+; CHECK: [[C4:%[0-9]+]](s32) = G_CONSTANT i32 4
+; CHECK: [[VEC:%[0-9]+]](<4 x s32>) = G_MERGE_VALUES [[C1]](s32), [[C2]](s32), [[C3]](s32), [[C4]](s32)
+; CHECK: %q0 = COPY [[VEC]](<4 x s32>)
+  ret <4 x i32> <i32 1, i32 2, i32 3, i32 4>
+}
+
+define <2 x double> @test_constantdatavector_v2f64() {
+; CHECK-LABEL: name: test_constantdatavector_v2f64
+; CHECK: [[FC1:%[0-9]+]](s64) = G_FCONSTANT double 1.000000e+00
+; CHECK: [[FC2:%[0-9]+]](s64) = G_FCONSTANT double 2.000000e+00
+; CHECK: [[VEC:%[0-9]+]](<2 x s64>) = G_MERGE_VALUES [[FC1]](s64), [[FC2]](s64)
+; CHECK: %q0 = COPY [[VEC]](<2 x s64>)
+  ret <2 x double> <double 1.0, double 2.0>
+}