[PATCH] D42784: [ARM] Allow Half types in ConstantPool

[Oliver Stannard via Phabricator via llvm-commits]

olista01 added inline comments.


================
Comment at: lib/Target/ARM/ARMConstantIslandPass.cpp:513
     unsigned Size = TD.getTypeAllocSize(CPs[i].getType());
-    assert(Size >= 4 && "Too small constant pool entry");
+    assert(Size >= 2 && "Too small constant pool entry");
     unsigned Align = CPs[i].getAlignment();
----------------
Is this assertion still useful?


================
Comment at: lib/Target/ARM/ARMConstantIslandPass.cpp:1432
+  // 16-bit value.
+  if (CPELogAlign == 1 && !isThumb1)
+    NewMBB->setAlignment(2);
----------------
It looks like this function is called when a BB must be split to insert a constant island, but not when extending an existing one. Will this fail if a BB is split to insert a 32-bit constant, and that island is then extended with a 16-bit constant?

Why is this being done for Thumb2?

Would it be easier to always setting the alignment of the new block to 1 for Thumb and 2 for ARM, so that this will work if we add 1-byte literal pools in future? I think that should also fix the issue of extending existing islands too.


https://reviews.llvm.org/D42784