[SPIR-V] Add pre-headers to loops. #75844
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-backend-spir-v Author: Nathan Gauër (Keenuts) ChangesThis is the first of the 7 steps outlined in #75801. This PR explicitely calls the SimplifyLoops pass. Directly following this pass should follow the 6 others required to structurize the IR. Running this pass could generate empty basic-blocks, which are implicit fallthrough to the successor BB. Full diff: https://github.com/llvm/llvm-project/pull/75844.diff 5 Files Affected:
diff --git a/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp b/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
index f4076be2a7b778..275c24e22c3025 100644
--- a/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
@@ -587,6 +587,40 @@ static void processSwitches(MachineFunction &MF, SPIRVGlobalRegistry *GR,
}
}
+static bool isImplicitFallthrough(MachineBasicBlock &MBB) {
+ if (MBB.empty())
+ return true;
+
+ // Branching spirv 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 spirv 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>();
@@ -599,6 +633,7 @@ bool SPIRVPreLegalizer::runOnMachineFunction(MachineFunction &MF) {
generateAssignInstrs(MF, GR, MIB);
processSwitches(MF, GR, MIB);
processInstrsWithTypeFolding(MF, GR, MIB);
+ removeImplicitFallthroughs(MF, MIB);
return true;
}
diff --git a/llvm/lib/Target/SPIRV/SPIRVTargetMachine.cpp b/llvm/lib/Target/SPIRV/SPIRVTargetMachine.cpp
index 1503f263e42c0d..517174b356f059 100644
--- a/llvm/lib/Target/SPIRV/SPIRVTargetMachine.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVTargetMachine.cpp
@@ -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;
@@ -151,6 +152,11 @@ TargetPassConfig *SPIRVTargetMachine::createPassConfig(PassManagerBase &PM) {
}
void SPIRVPassConfig::addIRPasses() {
+ // 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.
+ addPass(createLoopSimplifyPass());
+
TargetPassConfig::addIRPasses();
addPass(createSPIRVRegularizerPass());
addPass(createSPIRVPrepareFunctionsPass(TM));
diff --git a/llvm/lib/Target/SPIRV/SPIRVUtils.cpp b/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
index 1c0e8d84e2fd10..d4f7d8e89af5e4 100644
--- a/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVUtils.cpp
@@ -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;
}
diff --git a/llvm/lib/Target/SPIRV/SPIRVUtils.h b/llvm/lib/Target/SPIRV/SPIRVUtils.h
index 30fae6c7de479f..60742e2f272808 100644
--- a/llvm/lib/Target/SPIRV/SPIRVUtils.h
+++ b/llvm/lib/Target/SPIRV/SPIRVUtils.h
@@ -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);
diff --git a/llvm/test/CodeGen/SPIRV/scfg-add-pre-headers.ll b/llvm/test/CodeGen/SPIRV/scfg-add-pre-headers.ll
new file mode 100644
index 00000000000000..eccf3301002ae7
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/scfg-add-pre-headers.ll
@@ -0,0 +1,67 @@
+; RUN: llc -mtriple=spirv-unknown-unknown -O0 %s -o - | FileCheck %s
+
+; CHECK-DAG: [[bool:%[0-9]+]] = OpTypeBool
+; CHECK-DAG: [[uint:%[0-9]+]] = OpTypeInt 32 0
+; CHECK-DAG: [[uint_0:%[0-9]+]] = OpConstant [[uint]] 0
+
+define i32 @main(i32 noundef %0) #1 {
+ %2 = icmp ne i32 %0, 0
+ br i1 %2, label %l1, label %l2
+
+; CHECK: [[param_0:%[0-9]+]] = OpFunctionParameter [[uint]]
+; CHECK: [[cond:%[0-9]+]] = OpINotEqual [[bool]] [[param_0]] [[uint_0]]
+; CHECK: OpBranchConditional [[cond]] [[l1_pre:%[0-9]+]] [[l2_pre:%[0-9]+]]
+
+; CHECK-DAG: [[l2_pre]] = OpLabel
+; CHECK-NEXT: OpBranch [[l2_header:%[0-9]+]]
+
+; CHECK-DAG: [[l1_pre]] = OpLabel
+; CHECK-NEXT: OpBranch [[l1_header:%[0-9]+]]
+
+l1:
+ br i1 %2, label %l1_body, label %l1_end
+; CHECK-DAG: [[l1_header]] = OpLabel
+; CHECK-NEXT: OpBranchConditional [[cond]] [[l1_body:%[0-9]+]] [[l1_end:%[0-9]+]]
+
+l1_body:
+ br label %l1_continue
+; CHECK-DAG: [[l1_body]] = OpLabel
+; CHECK-NEXT: OpBranch [[l1_continue:%[0-9]+]]
+
+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:%[0-9]+]]
+
+l2:
+ br i1 %2, label %l2_body, label %l2_end
+; CHECK-DAG: [[l2_header]] = OpLabel
+; CHECK-NEXT: OpBranchConditional [[cond]] [[l2_body:%[0-9]+]] [[l2_end:%[0-9]+]]
+
+l2_body:
+ br label %l2_continue
+; CHECK-DAG: [[l2_body]] = OpLabel
+; CHECK-NEXT: OpBranch [[l2_continue:%[0-9]+]]
+
+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:%[0-9]+]]
+
+end:
+ ret i32 1
+; CHECK-DAG: [[end]] = OpLabel
+; CHECK-NEXT: OpReturn
+}
+
+attributes #1 = { "hlsl.numthreads"="4,8,16" "hlsl.shader"="compute" convergent }
|
Keenuts
requested review from
michalpaszkowski
and removed request for
iliya-diyachkov
|
@iliya-diyachkov don't know if you want to review, but FYI as this could changes the current codegen |
@Keenuts , do you think we should structure CFG for all SPIR-V flavors (both graphics and OpenCL)? If not, we probably need to call removeImplicitFallthroughs() and run createLoopSimplifyPass, checking for some flag or extension (or maybe isOpenCLEnv() from SPIRVSubtarget)? |
You are right, I should gate the added pass with a |
sudonatalie
approved these changes
Jan 2, 2024
| @@ -0,0 +1,67 @@ | |||
| ; RUN: llc -mtriple=spirv-unknown-unknown -O0 %s -o - | FileCheck %s | |||
There was a problem hiding this comment.
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 %}
There was a problem hiding this comment.
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.
iliya-diyachkov
approved these changes
Jan 4, 2024
michalpaszkowski
approved these changes
Jan 4, 2024
This is the first of the 7 steps outlined in llvm#75801. This PR explicitely calls the SimplifyLoops pass. Directly following this pass should follow the 6 others required to structurize the IR. Running this pass could generate empty basic-blocks, which are implicit fallthrough to the successor BB. There was a specific condition in the SPIR-V ISel which handled implicit fallthrough, but it couldn't work on empty basic-blocks. This commits removes the old logic, and adds this new logic, which checks all basic-blocks for implicit fallthroughs, including empty ones. Signed-off-by: Nathan Gauër <brioche@google.com>
only allow pass for Vulkan Signed-off-by: Nathan Gauër <brioche@google.com>
fix test format
fix test format
fix CHECK indent
review feedback
Keenuts
force-pushed
the
add-pre-headers
branch
from
c10a275
to
5b1ce3a
Compare
|
Rebased on main, check-all reported all green. Merging now |
justinfargnoli
pushed a commit
to justinfargnoli/llvm-project
that referenced
this pull request
Jan 28, 2024
This is the first of the 7 steps outlined in llvm#75801. This PR explicitely calls the SimplifyLoops pass. Directly following this pass should follow the 6 others required to structurize the IR. Running this pass could generate empty basic-blocks, which are implicit fallthrough to the successor BB. There was a specific condition in the SPIR-V ISel which handled implicit fallthrough, but it couldn't work on empty basic-blocks. This commits removes the old logic, and adds this new logic, which checks all basic-blocks for implicit fallthroughs, including empty ones. --------- Signed-off-by: Nathan Gauër <brioche@google.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the first of the 7 steps outlined in #75801. This PR explicitely calls the SimplifyLoops pass. Directly following this pass should follow the 6 others required to structurize the IR.
Running this pass could generate empty basic-blocks, which are implicit fallthrough to the successor BB.
There was a specific condition in the SPIR-V ISel which handled implicit fallthrough, but it couldn't work on empty basic-blocks. This commits removes the old logic, and adds this new logic, which checks all basic-blocks for implicit fallthroughs, including empty ones.