[CHERIoT] Teach LICM not to reorder GEP indices with different signs on CHERIoT.

Because of our tight representable bounds, any reordering of differently signed indices
is very likely to generate an invalid intermediate capability.

Author: resistor

llvm/lib/Transforms/Scalar/LICM.cpp

@@ -916,9 +916,9 @@ bool llvm::hoistRegion(DomTreeNode *N, AAResults *AA, LoopInfo *LI,
       // to that block.
       if (CurLoop->hasLoopInvariantOperands(&I) &&
           canSinkOrHoistInst(I, AA, DT, CurLoop, MSSAU, true, Flags, ORE) &&
-          isSafeToExecuteUnconditionally(
-              I, DT, TLI, CurLoop, SafetyInfo, ORE,
-              Preheader->getTerminator(), AC, AllowSpeculation)) {
+          isSafeToExecuteUnconditionally(I, DT, TLI, CurLoop, SafetyInfo, ORE,
+                                         Preheader->getTerminator(), AC,
+                                         AllowSpeculation)) {
         hoist(I, DT, CurLoop, CFH.getOrCreateHoistedBlock(BB), SafetyInfo,
               MSSAU, SE, ORE);
         HoistedInstructions.push_back(&I);
@@ -1035,8 +1035,8 @@ bool llvm::hoistRegion(DomTreeNode *N, AAResults *AA, LoopInfo *LI,
   if (VerifyMemorySSA)
     MSSAU.getMemorySSA()->verifyMemorySSA();
 
-    // Now that we've finished hoisting make sure that LI and DT are still
-    // valid.
+  // Now that we've finished hoisting make sure that LI and DT are still
+  // valid.
 #ifdef EXPENSIVE_CHECKS
   if (Changed) {
     assert(DT->verify(DominatorTree::VerificationLevel::Fast) &&
@@ -1137,7 +1137,7 @@ bool isOnlyMemoryAccess(const Instruction *I, const Loop *L,
     }
   return true;
 }
-}
+} // namespace
 
 static MemoryAccess *getClobberingMemoryAccess(MemorySSA &MSSA,
                                                BatchAAResults &BAA,
@@ -1186,8 +1186,8 @@ bool llvm::canSinkOrHoistInst(Instruction &I, AAResults *AA, DominatorTree *DT,
 
     bool InvariantGroup = LI->hasMetadata(LLVMContext::MD_invariant_group);
 
-    bool Invalidated = pointerInvalidatedByLoop(
-        MSSA, MU, CurLoop, I, Flags, InvariantGroup);
+    bool Invalidated =
+        pointerInvalidatedByLoop(MSSA, MU, CurLoop, I, Flags, InvariantGroup);
     // Check loop-invariant address because this may also be a sinkable load
     // whose address is not necessarily loop-invariant.
     if (ORE && Invalidated && CurLoop->isLoopInvariant(LI->getPointerOperand()))
@@ -1284,7 +1284,7 @@ static bool isTriviallyReplaceablePHI(const PHINode &PN, const Instruction &I) {
 
 /// Return true if the instruction is foldable in the loop.
 static bool isFoldableInLoop(const Instruction &I, const Loop *CurLoop,
-                         const TargetTransformInfo *TTI) {
+                             const TargetTransformInfo *TTI) {
   if (auto *GEP = dyn_cast<GetElementPtrInst>(&I)) {
     InstructionCost CostI =
         TTI->getInstructionCost(&I, TargetTransformInfo::TCK_SizeAndLatency);
@@ -1639,7 +1639,7 @@ static bool sink(Instruction &I, LoopInfo *LI, DominatorTree *DT,
   // the instruction.
   // First check if I is worth sinking for all uses. Sink only when it is worth
   // across all uses.
-  SmallSetVector<User*, 8> Users(I.user_begin(), I.user_end());
+  SmallSetVector<User *, 8> Users(I.user_begin(), I.user_end());
   for (auto *UI : Users) {
     auto *User = cast<Instruction>(UI);
 
@@ -1672,8 +1672,8 @@ static void hoist(Instruction &I, const DominatorTree *DT, const Loop *CurLoop,
   LLVM_DEBUG(dbgs() << "LICM hoisting to " << Dest->getNameOrAsOperand() << ": "
                     << I << "\n");
   ORE->emit([&]() {
-    return OptimizationRemark(DEBUG_TYPE, "Hoisted", &I) << "hoisting "
-                                                         << ore::NV("Inst", &I);
+    return OptimizationRemark(DEBUG_TYPE, "Hoisted", &I)
+           << "hoisting " << ore::NV("Inst", &I);
   });
 
   // Metadata can be dependent on conditions we are hoisting above.
@@ -2376,7 +2376,8 @@ static bool pointerInvalidatedByLoop(MemorySSA *MSSA, MemoryUse *MU,
     MemoryAccess *Source = getClobberingMemoryAccess(*MSSA, BAA, Flags, MU);
     return !MSSA->isLiveOnEntryDef(Source) &&
            CurLoop->contains(Source->getBlock()) &&
-           !(InvariantGroup && Source->getBlock() == CurLoop->getHeader() && isa<MemoryPhi>(Source));
+           !(InvariantGroup && Source->getBlock() == CurLoop->getHeader() &&
+             isa<MemoryPhi>(Source));
   }
 
   // For sinking, we'd need to check all Defs below this use. The getClobbering
@@ -2535,6 +2536,26 @@ static bool hoistGEP(Instruction &I, Loop &L, ICFLoopSafetyInfo &SafetyInfo,
                     all_of(Src->indices(), NonNegative) &&
                     all_of(GEP->indices(), NonNegative);
 
+  // CHERIoT has very tight representable bounds in its capability formatting,
+  // meaning that any intermediate out-of-bounds values during offsetting are
+  // likely to cause invalid pointers. As such, it require a much stricter
+  // condition to reorder indices than other targets, namely that the indices
+  // must be known either to be all non-negative, or all non-positive.
+  StringRef ABI = GEP->getModule()->getTargetABIFromMD();
+  bool MustStayInBounds = DL.isFatPointer(GEP->getType()) &&
+                          (ABI == "cheriot" || ABI == "cheriot-baremetal");
+  if (MustStayInBounds) {
+    auto NonPositive = [&](Value *V) {
+      return !isKnownPositive(V, SimplifyQuery(DL, DT, AC, GEP));
+    };
+    bool AllNonNeg = all_of(Src->indices(), NonNegative) &&
+                     all_of(GEP->indices(), NonNegative);
+    bool AllNonPos = all_of(Src->indices(), NonPositive) &&
+                     all_of(GEP->indices(), NonPositive);
+    if (!AllNonNeg && !AllNonPos)
+      return false;
+  }
+
   BasicBlock *Preheader = L.getLoopPreheader();
   IRBuilder<> Builder(Preheader->getTerminator());
   Value *NewSrc = Builder.CreateGEP(GEP->getSourceElementType(), SrcPtr,

llvm/test/Transforms/LICM/cheriot-gep-reorder.ll

@@ -0,0 +1,63 @@
+; RUN: opt < %s -S -passes=licm | FileCheck %s
+target datalayout = "e-m:e-p:32:32-i64:64-n32-S128-pf200:64:64:64:32-A200-P200-G200"
+target triple = "riscv32-unknown-cheriotrtos"
+
+; Because of the tightness of representable bounds on Cheriot, LICM must not reorder
+; GEP indices if they are not guaranteed to be the same sign.
+; CHECK-LABEL: @foo1
+; CHECK: entry:
+; CHECK-NOT: getelementptr
+; CHECK: do.body:
+; CHECK: getelementptr
+; CHECK-SAME: i32 %inc
+; CHECK: getelementptr
+; CHECK-SAME: i32 -8
+
+define void @foo1(ptr addrspace(200) noundef %r) {
+entry:
+  br label %do.body
+
+do.body:
+  %ctr.0 = phi i32 [ 0, %entry ], [ %inc, %do.body ]
+  %inc = add nuw nsw i32 %ctr.0, 1
+  %add.ptr = getelementptr inbounds nuw i16, ptr addrspace(200) %r, i32 %inc
+  %add.ptr1 = getelementptr inbounds i8, ptr addrspace(200) %add.ptr, i32 -8
+  tail call void @bar(ptr addrspace(200) noundef nonnull %add.ptr1)
+  %exitcond.not = icmp eq i32 %inc, 256
+  br i1 %exitcond.not, label %do.end, label %do.body
+
+do.end:
+  ret void
+}
+
+; CHECK-LABEL: @foo2
+; CHECK: entry:
+; CHECK: getelementptr
+; CHECK-SAME: i32 8
+; CHECK: do.body:
+; CHECK: getelementptr
+; CHECK-SAME: i32 %inc
+; CHECK-NOT: getelementptr
+
+define void @foo2(ptr addrspace(200) noundef %r) {
+entry:
+  br label %do.body
+
+do.body:
+  %ctr.0 = phi i32 [ 0, %entry ], [ %inc, %do.body ]
+  %inc = add nuw nsw i32 %ctr.0, 1
+  %add.ptr = getelementptr inbounds nuw i16, ptr addrspace(200) %r, i32 %inc
+  %add.ptr1 = getelementptr inbounds i8, ptr addrspace(200) %add.ptr, i32 8
+  tail call void @bar(ptr addrspace(200) noundef nonnull %add.ptr1)
+  %exitcond.not = icmp eq i32 %inc, 256
+  br i1 %exitcond.not, label %do.end, label %do.body
+
+do.end:
+  ret void
+}
+
+
+declare void @bar(ptr addrspace(200) noundef)
+
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"target-abi", !"cheriot"}