Skip to content

Commit

Permalink
[EraVM] Only generate GEPs that dominate latch BB in EraVMIndexedMemO…
Browse files Browse the repository at this point in the history
…psPrepare

In case we generate GEP that doesn't dominate latch BB,
we will run into an issue and verifier will complain:
```
Instruction does not dominate all uses!
  %3 = getelementptr inbounds i256, ptr addrspace(1) %0, i256 1
  %0 = phi ptr addrspace(1) [ %arg3, %bb1 ], [ %3, %bb5 ]
```
This patch fixes this issue.

Signed-off-by: Vladimir Radosavljevic <vr@matterlabs.dev>
  • Loading branch information
vladimirradosavljevic committed Aug 12, 2024
1 parent 6db0b6f commit 4da3cc2
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 1 deletion.
13 changes: 12 additions & 1 deletion llvm/lib/Target/EraVM/EraVMIndexedMemOpsPrepare.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
#include "llvm/Analysis/LoopPass.h"
#include "llvm/Analysis/ScalarEvolution.h"
#include "llvm/Analysis/ScalarEvolutionExpressions.h"
#include "llvm/IR/Dominators.h"
#include "llvm/IR/IRBuilder.h"
#include "llvm/InitializePasses.h"
#include "llvm/Support/CommandLine.h"
Expand All @@ -85,7 +86,7 @@ using namespace llvm;
namespace {

class EraVMIndexedMemOpsPrepare : public LoopPass {

DominatorTree *DT = nullptr;
ScalarEvolution *SE = nullptr;
LLVMContext *Ctx = nullptr;
Loop *CurrentLoop = nullptr;
Expand All @@ -102,8 +103,10 @@ class EraVMIndexedMemOpsPrepare : public LoopPass {
}

void getAnalysisUsage(AnalysisUsage &AU) const override {
AU.addRequired<DominatorTreeWrapperPass>();
AU.addRequired<ScalarEvolutionWrapperPass>();
AU.addRequired<LoopInfoWrapperPass>();
AU.addPreserved<DominatorTreeWrapperPass>();
AU.addPreserved<LoopInfoWrapperPass>();
AU.setPreservesCFG();
}
Expand Down Expand Up @@ -208,6 +211,12 @@ bool EraVMIndexedMemOpsPrepare::isValidGEPAndIncByOneCell(
if (!PHI)
return false;

// The PHI node must be in the loop header, and BasePtr BB must dominate
// the latch BB.
if (CurrentLoop->getHeader() != PHI->getParent() ||
!DT->dominates(BasePtr->getParent(), CurrentLoop->getLoopLatch()))
return false;

// In rewriteToFavorIndexedMemOps, new PHI is created with incoming values
// from preheader and latch. Check that this PHI has the same incoming BBs.
return PHI->getNumIncomingValues() == 2 &&
Expand All @@ -225,6 +234,7 @@ bool EraVMIndexedMemOpsPrepare::runOnLoop(Loop *L, LPPassManager &) {
return false;

auto &LI = getAnalysis<LoopInfoWrapperPass>().getLoopInfo();
DT = &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
SE = &getAnalysis<ScalarEvolutionWrapperPass>().getSE();
Ctx = &L->getLoopPreheader()->getContext();
CurrentLoop = L;
Expand Down Expand Up @@ -268,6 +278,7 @@ char EraVMIndexedMemOpsPrepare::ID = 0;

INITIALIZE_PASS_BEGIN(EraVMIndexedMemOpsPrepare, DEBUG_TYPE,
ERAVM_PREPARE_INDEXED_MEMOPS_NAME, false, false)
INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
INITIALIZE_PASS_DEPENDENCY(ScalarEvolutionWrapperPass)
INITIALIZE_PASS_DEPENDENCY(LoopInfoWrapperPass)
INITIALIZE_PASS_END(EraVMIndexedMemOpsPrepare, DEBUG_TYPE,
Expand Down
43 changes: 43 additions & 0 deletions llvm/test/CodeGen/EraVM/indexed-memops-gep-dominate-bug.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
; RUN: llc -O3 -stop-after=verify -compile-twice=false < %s | FileCheck %s

target datalayout = "E-p:256:256-i256:256:256-S32-a:256:256"
target triple = "eravm"

define i256 @test(i256 %arg, ptr addrspace(1) %arg1, ptr addrspace(1) %arg2) {
; CHECK-LABEL: @test(
entry:
%icmp1 = icmp eq i256 %arg, 0
br i1 %icmp1, label %bb7, label %bb1

bb1:
br label %bb2

bb2:
%phi1 = phi i256 [ %add, %bb5 ], [ 0, %bb1 ]
%icmp2 = icmp ugt i256 %phi1, 100
br i1 %icmp2, label %bb3, label %bb4

bb3:
; CHECK: getelementptr inbounds i256, ptr addrspace(1) %arg1, i256 %phi1
%gep1 = getelementptr inbounds i256, ptr addrspace(1) %arg1, i256 %phi1
store i256 5, ptr addrspace(1) %gep1, align 32
br label %bb5

bb4:
; CHECK: getelementptr inbounds i256, ptr addrspace(1) %arg2, i256 %phi1
%gep2 = getelementptr inbounds i256, ptr addrspace(1) %arg2, i256 %phi1
store i256 10, ptr addrspace(1) %gep2, align 32
br label %bb5

bb5:
%add = add i256 %phi1, 1
%cmp3 = icmp eq i256 %add, 0
br i1 %cmp3, label %bb6, label %bb2

bb6:
br label %bb7

bb7:
%phi2 = phi i256 [ 0, %entry ], [ %add, %bb6 ]
ret i256 %phi2
}

0 comments on commit 4da3cc2

Please sign in to comment.