Skip to content

[OMPIRBuilder][OpenMP][LLVM] Modify and use ReplaceConstant utility in convertTarget #94541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion llvm/include/llvm/IR/ReplaceConstant.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,13 @@ namespace llvm {

template <typename T> class ArrayRef;
class Constant;
class Function;

/// Replace constant expressions users of the given constants with
/// instructions. Return whether anything was changed.
bool convertUsersOfConstantsToInstructions(ArrayRef<Constant *> Consts);
bool convertUsersOfConstantsToInstructions(ArrayRef<Constant *> Consts,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add descriptions of the new parameters to the function in the comment above.

Function *RestrictToFunc = nullptr,
bool RemoveDeadConstants = true);

} // end namespace llvm

Expand Down
48 changes: 17 additions & 31 deletions llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
#include "llvm/IR/MDBuilder.h"
#include "llvm/IR/Metadata.h"
#include "llvm/IR/PassManager.h"
#include "llvm/IR/ReplaceConstant.h"
#include "llvm/IR/Value.h"
#include "llvm/MC/TargetRegistry.h"
#include "llvm/Support/CommandLine.h"
Expand Down Expand Up @@ -5092,27 +5093,6 @@ FunctionCallee OpenMPIRBuilder::createDispatchFiniFunction(unsigned IVSize,
return getOrCreateRuntimeFunction(M, Name);
}

static void replaceConstatExprUsesInFuncWithInstr(ConstantExpr *ConstExpr,
Function *Func) {
for (User *User : make_early_inc_range(ConstExpr->users())) {
if (auto *Instr = dyn_cast<Instruction>(User)) {
if (Instr->getFunction() == Func) {
Instruction *ConstInst = ConstExpr->getAsInstruction();
ConstInst->insertBefore(*Instr->getParent(), Instr->getIterator());
Instr->replaceUsesOfWith(ConstExpr, ConstInst);
}
}
}
}

static void replaceConstantValueUsesInFuncWithInstr(llvm::Value *Input,
Function *Func) {
for (User *User : make_early_inc_range(Input->users()))
if (auto *Const = dyn_cast<Constant>(User))
if (auto *ConstExpr = dyn_cast<ConstantExpr>(Const))
replaceConstatExprUsesInFuncWithInstr(ConstExpr, Func);
}

static Function *createOutlinedFunction(
OpenMPIRBuilder &OMPBuilder, IRBuilderBase &Builder, StringRef FuncName,
SmallVectorImpl<Value *> &Inputs,
Expand Down Expand Up @@ -5191,17 +5171,23 @@ static Function *createOutlinedFunction(

// Things like GEP's can come in the form of Constants. Constants and
// ConstantExpr's do not have access to the knowledge of what they're
// contained in, so we must dig a little to find an instruction so we can
// tell if they're used inside of the function we're outlining. We also
// replace the original constant expression with a new instruction
// contained in, so we must dig a little to find an instruction so we
// can tell if they're used inside of the function we're outlining. We
// also replace the original constant expression with a new instruction
// equivalent; an instruction as it allows easy modification in the
// following loop, as we can now know the constant (instruction) is owned by
// our target function and replaceUsesOfWith can now be invoked on it
// (cannot do this with constants it seems). A brand new one also allows us
// to be cautious as it is perhaps possible the old expression was used
// inside of the function but exists and is used externally (unlikely by the
// nature of a Constant, but still).
replaceConstantValueUsesInFuncWithInstr(Input, Func);
// following loop, as we can now know the constant (instruction) is
// owned by our target function and replaceUsesOfWith can now be invoked
// on it (cannot do this with constants it seems). A brand new one also
// allows us to be cautious as it is perhaps possible the old expression
// was used inside of the function but exists and is used externally
// (unlikely by the nature of a Constant, but still).
// NOTE: We cannot remove dead constants that have been rewritten to
// instructions at this stage, we run the risk of breaking later lowering
// by doing so as we could still be in the process of lowering the module
// from MLIR to LLVM-IR and the MLIR lowering may still require the original
// constants we have created rewritten versions of.
if (auto *Const = dyn_cast<Constant>(Input))
convertUsersOfConstantsToInstructions({Const}, Func, false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
convertUsersOfConstantsToInstructions({Const}, Func, false);
convertUsersOfConstantsToInstructions(Const, Func, false);

ArrayRef has a single-value overload.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thank you, I wasn't aware! So used to brace initializing most things. I'll make this alteration and update in a little bit :-)


// Collect all the instructions
for (User *User : make_early_inc_range(Input->users()))
Expand Down
12 changes: 8 additions & 4 deletions llvm/lib/IR/ReplaceConstant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ static SmallVector<Instruction *, 4> expandUser(BasicBlock::iterator InsertPt,
return NewInsts;
}

bool convertUsersOfConstantsToInstructions(ArrayRef<Constant *> Consts) {
bool convertUsersOfConstantsToInstructions(ArrayRef<Constant *> Consts,
Function *RestrictToFunc,
bool RemoveDeadConstants) {
// Find all expandable direct users of Consts.
SmallVector<Constant *> Stack;
for (Constant *C : Consts)
Expand All @@ -74,7 +76,8 @@ bool convertUsersOfConstantsToInstructions(ArrayRef<Constant *> Consts) {
for (Constant *C : ExpandableUsers)
for (User *U : C->users())
if (auto *I = dyn_cast<Instruction>(U))
InstructionWorklist.insert(I);
if (!RestrictToFunc || I->getFunction() == RestrictToFunc)
InstructionWorklist.insert(I);

// Replace those expandable operands with instructions
bool Changed = false;
Expand Down Expand Up @@ -102,8 +105,9 @@ bool convertUsersOfConstantsToInstructions(ArrayRef<Constant *> Consts) {
}
}

for (Constant *C : Consts)
C->removeDeadConstantUsers();
if (RemoveDeadConstants)
for (Constant *C : Consts)
C->removeDeadConstantUsers();

return Changed;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,11 @@ module attributes {omp.is_target_device = false} {

// CHECK: define void @_QQmain()
// CHECK: %[[SCALAR_ALLOCA:.*]] = alloca { ptr, i64, i32, i8, i8, i8, i8 }, i64 1, align 8
// CHECK: %[[FULL_ARR_SIZE5:.*]] = load i64, ptr getelementptr ({ ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]] }, ptr @[[FULL_ARR_GLOB]], i32 0, i32 7, i64 0, i32 1), align 4
// CHECK: %[[FULL_ARR_GEP:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]] }, ptr @[[FULL_ARR_GLOB]], i32 0, i32 7, i64 0, i32 1
// CHECK: %[[FULL_ARR_SIZE5:.*]] = load i64, ptr %[[FULL_ARR_GEP]], align 4
// CHECK: %[[FULL_ARR_SIZE4:.*]] = sub i64 %[[FULL_ARR_SIZE5]], 1
// CHECK: %[[ARR_SECT_OFFSET3:.*]] = load i64, ptr getelementptr ({ ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]] }, ptr @[[ARR_SECT_GLOB]], i32 0, i32 7, i64 0, i32 0), align 4
// CHECK: %[[ARR_SECT_GEP:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8, [1 x [3 x i64]] }, ptr @[[ARR_SECT_GLOB]], i32 0, i32 7, i64 0, i32 0
// CHECK: %[[ARR_SECT_OFFSET3:.*]] = load i64, ptr %[[ARR_SECT_GEP]], align 4
// CHECK: %[[ARR_SECT_OFFSET2:.*]] = sub i64 2, %[[ARR_SECT_OFFSET3]]
// CHECK: %[[ARR_SECT_SIZE4:.*]] = sub i64 5, %[[ARR_SECT_OFFSET3]]
// CHECK: %[[SCALAR_BASE:.*]] = getelementptr { ptr, i64, i32, i8, i8, i8, i8 }, ptr %[[SCALAR_ALLOCA]], i32 0, i32 0
Expand Down
51 changes: 51 additions & 0 deletions offload/test/offloading/fortran/dtype-array-constant-index-map.f90
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
! Offloading test which maps a specific element of a
! derived type to the device and then accesses the
! element alongside an individual element of an array
! that the derived type contains. In particular, this
! test helps to check that we can replace the constants
! within the kernel with instructions and then replace
! these instructions with the kernel parameters.
! REQUIRES: flang
! UNSUPPORTED: nvptx64-nvidia-cuda
! UNSUPPORTED: nvptx64-nvidia-cuda-LTO
! UNSUPPORTED: aarch64-unknown-linux-gnu
! UNSUPPORTED: aarch64-unknown-linux-gnu-LTO
! UNSUPPORTED: x86_64-pc-linux-gnu
! UNSUPPORTED: x86_64-pc-linux-gnu-LTO

! RUN: %libomptarget-compile-fortran-run-and-check-generic
module test_0
type dtype
integer elements(20)
integer value
end type dtype

type (dtype) array_dtype(5)
contains

subroutine assign()
implicit none
!$omp target map(tofrom: array_dtype(5))
array_dtype(5)%elements(5) = 500
!$omp end target
end subroutine

subroutine add()
implicit none

!$omp target map(tofrom: array_dtype(5))
array_dtype(5)%elements(5) = array_dtype(5)%elements(5) + 500
!$omp end target
end subroutine
end module test_0

program main
use test_0

call assign()
call add()

print *, array_dtype(5)%elements(5)
end program

! CHECK: 1000
Loading