[PATCH] D102830: [AMDGPU] Avoid null export insertion when unifying exit blocks
Carl Ritson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri May 21 03:16:16 PDT 2021
critson added inline comments.
================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUUnifyDivergentExitNodes.cpp:154
-BasicBlock *AMDGPUUnifyDivergentExitNodes::unifyReturnBlockSet(
- Function &F, DomTreeUpdater &DTU, ArrayRef<BasicBlock *> ReturningBlocks,
- bool InsertExport, StringRef Name) {
- // Otherwise, we need to insert a new basic block into the function, add a PHI
- // nodes (if the function returns values), and convert all of the return
- // instructions into unconditional branches.
- BasicBlock *NewRetBlock = BasicBlock::Create(F.getContext(), Name, &F);
- IRBuilder<> B(NewRetBlock);
+// Search a block and its predecessors to find a "done" export
+static IntrinsicInst *findExportDone(BasicBlock *BB,
----------------
foad wrote:
> critson wrote:
> > foad wrote:
> > > Isn't it possible that different predecessors have different "done" exports? What is this function supposed to do in that case?
> > This does assumes the IR is well-formed to a certain degree.
> > I have added a comment to try to document this.
> >
> > Consider the following IR:
> > ```if (condition) {
> > export done 1
> > } else {
> > export done 2
> > }
> > return```
> >
> > This will eventually be compiled to:
> > ```set exec mask for if-branch
> > export done 1
> > set exec mask for else-branch
> > export done 2
> > restore exec mask
> > return```
> >
> > Multiple export done instructions will be executed which is invalid.
> > The expected cases for this are:
> >
> > 1. Divergent exits with their own exports, e.g.
> > ```if (condition) {
> > export done 1
> > return
> > } else {
> > export done 2
> > return
> > }```
> >
> > 2. Or divergent exits without exports or a uniformly reached export, e.g.
> > ```export done
> > if (condition) {
> > return
> > } else {
> > return
> > }```
> OK. Thanks for explaining. I find it hard to get my head round the possible cases that we do or don't have to handle. Do you cope with:
> ```
> if (uniform condition)
> export done 1
> else
> export done 2
> if (divergent condition)
> return 1
> else
> return 2
> ```
> ?
Good question, I think the answer is no -- although it is in principle valid IR that we do not generate.
I realize I was thinking primarily of divergent conditions, rather than uniform ones.
Although the comment I added is still accurate, <= 1 reachable export done.
In principle I can rework this to handle the case you raised, but I am not sure it is worth it?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102830/new/
https://reviews.llvm.org/D102830
More information about the llvm-commits
mailing list