[PATCH] D119475: [AMDGPU] Add scheduler pass to rematerialize trivial defs

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/GCNSchedStrategy.cpp:830
+      // not need to check RegionEnd since it can only be a call or terminator
+      // instruction and we do not touch those type of instructions.
+      for (unsigned DefI = 0, DefE = Regions.size(); DefI != DefE; ++DefI) {
----------------
vangthao wrote:
> rampitec wrote:
> > rampitec wrote:
> > > vangthao wrote:
> > > > rampitec wrote:
> > > > > rampitec wrote:
> > > > > > You actually need to update RegionEnd because there can be a fallthrough without a terminator.
> > > > > And actually remember that region is not the same as basic block. It may start and end in the middle of a block.
> > > > Can you give an example of a fallthrough case? I thought regions were decided by isSchedBoundary().
> > > A basic block may finish without a terminator. It will just fall through into the next block.
> > Look at the dbg-value-ends-sched-region.mir for example. The first block is ended with a V_MOV_B32:
> > ```
> >     %10:vreg_64 = IMPLICIT_DEF
> >     undef %11.sub0:vreg_64 = V_MOV_B32_e32 0, implicit $exec
> > 
> >   bb.1:
> >     %12:vreg_64 = COPY %2
> > ```
> > So if you move this instruction you will invalidate RegionEnd.
> I believe RegionEnd would be equivalent to MBB.end() which is a pass the end iterator  and would not point to the V_MOV_B32 in this case, or am I wrong?
OK, makes sense. None of isSchedulingBoundary() instructions are rematerializable too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119475/new/

https://reviews.llvm.org/D119475