[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 2 inline comments as done.
simpal01 added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:1390
       MI = LastIT;
   }
 
----------------
dmgreen wrote:
> Would it be possible to put the loop here? So if we didn't see a LastIT, but are //still// in a IT block, we need to get out of it one way or another. My understanding it that would only happen if we started in an IT block (but may be mistaken).
Yes. It happened to notice only when we did not see a LastIT, but are still in a IT block


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


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

https://reviews.llvm.org/D64621