[PATCH] D64621: [ARM] Make sure that the constant pool does not keep in the middle of an IT block.

[Simi Pallipurath via Phabricator via llvm-commits]

simpal01 marked an inline comment as done.
simpal01 added inline comments.


================
Comment at: llvm/test/CodeGen/ARM/constant-islands-split-IT.mir:93
+    renamable $d0 = VLDRD %const.1, 0, 0, $cpsr, implicit $itstate :: (load 8 from constant-pool)
+    renamable $d1 = VLDRD %const.2, 0, 0, $cpsr, implicit $itstate :: (load 8 from constant-pool)
+    renamable $d2 = VLDRD %const.0, 0, 0, $cpsr, implicit $itstate :: (load 8 from constant-pool)
----------------
simpal01 wrote:
> efriedma wrote:
> > Do you know why we're trying to split this block in the first place?  It should be possible to place all the necessary constant pool entries after the call to __stack_chk_fail.
> 
> When this pass iteratively placing the constant pool placements, 
> 
>    1. If the CPE is placed after the water then that water get removed from the WaterList.
>    2. There is a NewWaterList which will be updated with the NewIsland created.
>    3. There is something called HighWaterMark which records the highest basic block where a CPEntry is placed.
> 
> In this particular test case, after the first iteration of constant pool placements, the block structure will be looking like below.The CONSTPOOL_ENTRY_6, %const.0  is added after the UserBB "BB5" because that is the only place where it can find the water in range.
> 
> BBO:
>      Referring %const.0
> BB1:
>    CONSTPOOL_ENTRY_3 (%const.0) 
> BB2:
>    CONSTPOOL_ENTRY_4 (%const.1)
> BB3:
>    CONSTPOOL_ENTRY_5 (%const.2)
> 
> BB4: SPACE 790
> 
> BB5:
>     Referring %const.1
>     Referring %const.2
>     Referring %const.0
> 
> BB6:
>     CONSTPOOL_ENTRY_6 (%const.0)  
> 
> When the second iteration starts, the current placement for the CP entry CONSTPOOL_ENTRY_4 (%const.1) for the CPUser in  BB5 will be out of range.Then it tries to look for water where it can place this CPE. It can see BB6 is in range. But it not only checks water is in range but also checks if it is either at a lower address than the high water mark (this is not  true here.The current high watermark for this CPE is 2 which is greater than BB6) or a new water block that was created (BB6 is the new water block that was created in the end  of previous iteration,but the NewWaterList gets cleared before the second iteration started) . Hence it can not find any water where it can place CONSTPOOL_ENTRY_4 (%const.1).  Then it tries to split in the middle of the UserBB BB5.
just a small correction in the above writings. 

>> if it is either at a lower address than the high water mark (this is not true here.The current high watermark for this CPE is 2 which is greater than BB6) 

It should be,

>>if it is either at a lower address than the high water mark (this is not true here.The current HighWaterMark for this CPE is at BB2 which is already at lower address than BB6)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64621/new/

https://reviews.llvm.org/D64621