Skip to content

[SPIR-V] Add pre-headers to loops. #75844

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 7 commits into from
Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
35 changes: 35 additions & 0 deletions llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,40 @@ static void processSwitches(MachineFunction &MF, SPIRVGlobalRegistry *GR,
}
}

static bool isImplicitFallthrough(MachineBasicBlock &MBB) {
if (MBB.empty())
return true;

// Branching SPIR-V intrinsics are not detected by this generic method.
// Thus, we can only trust negative result.
if (!MBB.canFallThrough())
return false;

// Otherwise, we must manually check if we have a SPIR-V intrinsic which
// prevent an implicit fallthrough.
for (MachineBasicBlock::reverse_iterator It = MBB.rbegin(), E = MBB.rend();
It != E; ++It) {
if (isSpvIntrinsic(*It, Intrinsic::spv_switch))
return false;
}
return true;
}

static void removeImplicitFallthroughs(MachineFunction &MF,
MachineIRBuilder MIB) {
// It is valid for MachineBasicBlocks to not finish with a branch instruction.
// In such cases, they will simply fallthrough their immediate successor.
for (MachineBasicBlock &MBB : MF) {
if (!isImplicitFallthrough(MBB))
continue;

assert(std::distance(MBB.successors().begin(), MBB.successors().end()) ==
1);
MIB.setInsertPt(MBB, MBB.end());
MIB.buildBr(**MBB.successors().begin());
}
}

bool SPIRVPreLegalizer::runOnMachineFunction(MachineFunction &MF) {
// Initialize the type registry.
const SPIRVSubtarget &ST = MF.getSubtarget<SPIRVSubtarget>();
Expand All @@ -619,6 +653,7 @@ bool SPIRVPreLegalizer::runOnMachineFunction(MachineFunction &MF) {
generateAssignInstrs(MF, GR, MIB);
processSwitches(MF, GR, MIB);
processInstrsWithTypeFolding(MF, GR, MIB);
removeImplicitFallthroughs(MF, MIB);

return true;
}
Expand Down
14 changes: 14 additions & 0 deletions llvm/lib/Target/SPIRV/SPIRVTargetMachine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "llvm/MC/TargetRegistry.h"
#include "llvm/Pass.h"
#include "llvm/Target/TargetOptions.h"
#include "llvm/Transforms/Utils.h"
#include <optional>

using namespace llvm;
Expand Down Expand Up @@ -151,6 +152,19 @@ TargetPassConfig *SPIRVTargetMachine::createPassConfig(PassManagerBase &PM) {
}

void SPIRVPassConfig::addIRPasses() {
if (TM.getSubtargetImpl()->isVulkanEnv()) {
// Once legalized, we need to structurize the CFG to follow the spec.
// This is done through the following 8 steps.
// TODO(#75801): add the remaining steps.

// 1. Simplify loop for subsequent transformations. After this steps, loops
// have the following properties:
// - loops have a single entry edge (pre-header to loop header).
// - all loop exits are dominated by the loop pre-header.
// - loops have a single back-edge.
addPass(createLoopSimplifyPass());
}

TargetPassConfig::addIRPasses();
addPass(createSPIRVRegularizerPass());
addPass(createSPIRVPrepareFunctionsPass(TM));
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/SPIRV/SPIRVUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -228,8 +228,8 @@ uint64_t getIConstVal(Register ConstReg, const MachineRegisterInfo *MRI) {
return MI->getOperand(1).getCImm()->getValue().getZExtValue();
}

bool isSpvIntrinsic(MachineInstr &MI, Intrinsic::ID IntrinsicID) {
if (auto *GI = dyn_cast<GIntrinsic>(&MI))
bool isSpvIntrinsic(const MachineInstr &MI, Intrinsic::ID IntrinsicID) {
if (const auto *GI = dyn_cast<GIntrinsic>(&MI))
return GI->is(IntrinsicID);
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/SPIRV/SPIRVUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ MachineInstr *getDefInstrMaybeConstant(Register &ConstReg,
uint64_t getIConstVal(Register ConstReg, const MachineRegisterInfo *MRI);

// Check if MI is a SPIR-V specific intrinsic call.
bool isSpvIntrinsic(MachineInstr &MI, Intrinsic::ID IntrinsicID);
bool isSpvIntrinsic(const MachineInstr &MI, Intrinsic::ID IntrinsicID);

// Get type of i-th operand of the metadata node.
Type *getMDOperandAsType(const MDNode *N, unsigned I);
Expand Down
66 changes: 66 additions & 0 deletions llvm/test/CodeGen/SPIRV/scfg-add-pre-headers.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
; RUN: llc -mtriple=spirv-unknown-unknown -O0 %s -o - | FileCheck %s
Copy link
Member

Choose a reason for hiding this comment

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

Assuming this is a simple case that's already structured and validates, can you add a RUN line for spirv-val?

; RUN: %if spirv-tools %{ llc -mtriple=spirv-unknown-unknown -O0 %s -o - -filetype=obj | spirv-val %}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed the test to generate "valid" SPIR-V (function returned an int instead of void, etc), but I cannot add this line just yet. There are branches, and no OpSelectionMerge/OpLoopMerge yet, so this is not a valid SPIR-V module.


; CHECK-DAG: %[[#bool:]] = OpTypeBool
; CHECK-DAG: %[[#uint:]] = OpTypeInt 32 0
; CHECK-DAG: %[[#uint_0:]] = OpConstant %[[#uint]] 0

define void @main() #1 {
%1 = icmp ne i32 0, 0
br i1 %1, label %l1, label %l2

; CHECK: %[[#cond:]] = OpINotEqual %[[#bool]] %[[#uint_0]] %[[#uint_0]]
; CHECK: OpBranchConditional %[[#cond]] %[[#l1_pre:]] %[[#l2_pre:]]

; CHECK-DAG: %[[#l2_pre]] = OpLabel
; CHECK-NEXT: OpBranch %[[#l2_header:]]

; CHECK-DAG: %[[#l1_pre]] = OpLabel
; CHECK-NEXT: OpBranch %[[#l1_header:]]

l1:
br i1 %1, label %l1_body, label %l1_end
; CHECK-DAG: %[[#l1_header]] = OpLabel
; CHECK-NEXT: OpBranchConditional %[[#cond]] %[[#l1_body:]] %[[#l1_end:]]

l1_body:
br label %l1_continue
; CHECK-DAG: %[[#l1_body]] = OpLabel
; CHECK-NEXT: OpBranch %[[#l1_continue:]]

l1_continue:
br label %l1
; CHECK-DAG: %[[#l1_continue]] = OpLabel
; CHECK-NEXT: OpBranch %[[#l1_header]]

l1_end:
br label %end
; CHECK-DAG: %[[#l1_end]] = OpLabel
; CHECK-NEXT: OpBranch %[[#end:]]

l2:
br i1 %1, label %l2_body, label %l2_end
; CHECK-DAG: %[[#l2_header]] = OpLabel
; CHECK-NEXT: OpBranchConditional %[[#cond]] %[[#l2_body:]] %[[#l2_end:]]

l2_body:
br label %l2_continue
; CHECK-DAG: %[[#l2_body]] = OpLabel
; CHECK-NEXT: OpBranch %[[#l2_continue:]]

l2_continue:
br label %l2
; CHECK-DAG: %[[#l2_continue]] = OpLabel
; CHECK-NEXT: OpBranch %[[#l2_header]]

l2_end:
br label %end
; CHECK-DAG: %[[#l2_end]] = OpLabel
; CHECK-NEXT: OpBranch %[[#end:]]

end:
ret void
; CHECK-DAG: %[[#end]] = OpLabel
; CHECK-NEXT: OpReturn
}

attributes #1 = { "hlsl.numthreads"="4,8,16" "hlsl.shader"="compute" convergent }