[PATCH] D15264: Fix bitcast to gep constant folding transform.

[Amaury SECHET via llvm-commits]

deadalnix added inline comments.

================
Comment at: unittests/IR/ConstantsTest.cpp:387-398
@@ +386,14 @@
+  LLVMContext Context;
+  std::unique_ptr<Module> M(new Module("MyModule", Context));
+
+  auto *i32 = Type::getInt32Ty(Context);
+  Type *S1Elts[] = {i32, i32};
+  auto *S1 = StructType::create(S1Elts);
+
+  auto *G1 = new GlobalVariable(*M, S1, false,
+                                GlobalValue::ExternalLinkage, nullptr);
+  auto *PtrTy = PointerType::get(i32, 0);
+  auto *C1 = ConstantExpr::getBitCast(G1, PtrTy);
+  ASSERT_EQ(dyn_cast<ConstantExpr>(C1)->getOpcode(),
+            Instruction::GetElementPtr);
+
----------------
majnemer wrote:
> This test doesn't seem related to the change in question.  It worked before your change.
It doesn't looks like it is well tested.

================
Comment at: unittests/IR/ConstantsTest.cpp:400-408
@@ +399,11 @@
+
+  auto *U = StructType::create(Context, "Unsized");
+  Type *S2Elts[] = {i32, U};
+  auto *S2 = StructType::create(S2Elts);
+
+  auto *G2 = new GlobalVariable(*M, S2, false,
+                                GlobalValue::ExternalLinkage, nullptr);
+  auto *C2 = ConstantExpr::getBitCast(G2, PtrTy);
+  ASSERT_EQ(dyn_cast<ConstantExpr>(C2)->getOpcode(),
+            Instruction::BitCast);
+}
----------------
majnemer wrote:
> I'd move this test to test/Assembler/ConstantExprFoldCast.ll
> 
> The IR translation of this test would be something like:
> 
> ```
> %Unsized = type opaque
> %S2 = type { i32, %Unsized }
> @G2 = external global %S2
> @G3 = global i32* bitcast (%S2* @G2 to i32*)
> ```
The file check that there is no cast. I can put the first part of the test in there, but it doesn't looks like it is adequate for checking that this doesn't assert fail.


http://reviews.llvm.org/D15264