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

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:1372
+    // blocks it creates is not in the middle of an IT block.
+    while (MI->getOpcode() != ARM::t2IT &&
+           getITInstrPredicate(*MI, PredReg) != ARMCC::AL) {
----------------
I'm not sure I understand this (existing) code completely. Would it be possible for this new loop to walk _past_ the end of BaseInsertOffset, making a CPE out of range of the user? Because each iteration of this outer loop can now increase Offset by more than just 2/4 (getInstSizeInBytes), and the --MI at the end of the loop might not get us back in-range.

Or would it be possible to miss instructions that should be handled in the "if (CPUIndex < NumCPUsers && CPUsers[CPUIndex].MI == &*MI)" block above because they are inside IT blocks?


================
Comment at: llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:1390
       MI = LastIT;
   }
 
----------------
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).


================
Comment at: llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:1400
   // We really must not split an IT block.
   LLVM_DEBUG(unsigned PredReg; assert(
                  !isThumb || getITInstrPredicate(*MI, PredReg) == ARMCC::AL));
----------------
This assert shouldn't be in a LLVM_DEBUG. Can you move it out whilst you are here?


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

https://reviews.llvm.org/D64621