[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
llvm-commits at lists.llvm.org
Fri Jul 12 09:56:44 PDT 2019
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
More information about the llvm-commits
mailing list