[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:1432
+  // 16-bit value.
+  if (CPELogAlign == 1 && !isThumb1)
+    NewMBB->setAlignment(2);
----------------
olista01 wrote:
> 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.
This is still only going to be triggered if the _first_ entry in the constant pool is 2-byte, I think it will fail if the first entry is 4-byte, and the last is 2-byte. Would it also be possible to add a test for this case?


https://reviews.llvm.org/D42784